RE: Failed transaction statistics to measure the logical replication progress

From: "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>
To: 'vignesh C' <vignesh21(at)gmail(dot)com>
Cc: Greg Nancarrow <gregn4422(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Failed transaction statistics to measure the logical replication progress
Date: 2021-11-15 09:27:51
Message-ID: TYCPR01MB8373FEB287F733C81C1E4D42ED989@TYCPR01MB8373.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thursday, November 11, 2021 9:47 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> Few more comments:
> 1) Here the tuple length is not considered in the calculation, else it will always
> show the fixed size for any size tuple. Ex varchar insert with 1 byte or varchar
> insert with 100's of bytes. So I feel we should include the tuple length in the
> calculation.
> + case LOGICAL_REP_MSG_INSERT:
> + case LOGICAL_REP_MSG_UPDATE:
> + case LOGICAL_REP_MSG_DELETE:
> + Assert(extra_data != NULL);
> +
> + /*
> + * Compute size based on ApplyExecutionData.
> + * The size of LogicalRepRelMapEntry can be
> skipped because
> + * it is obtained from hash_search in
> logicalrep_rel_open.
> + */
> + size += sizeof(ApplyExecutionData) + sizeof(EState)
> +
> + sizeof(ResultRelInfo) +
> + sizeof(ResultRelInfo);
> +
> + /*
> + * Add some extra size if the target relation
> is partitioned.
> + * PartitionTupleRouting isn't exported.
> Therefore, call the
> + * function that returns its size instead.
> + */
> + if
> (extra_data->relmapentry->localrel->rd_rel->relkind ==
> RELKIND_PARTITIONED_TABLE)
> + size += sizeof(ModifyTableState) +
> PartitionTupleRoutingSize();
> + break;
Thanks a lot ! Fixed.

> 2) Can this be part of PgStat_StatDBEntry, similar to tables, functions and
> subworkers. It might be more appropriate to have it there instead of having
> another global variable.
> + * Stats of prepared transactions should be displayed
> + * at either commit prepared or rollback prepared time, even when it's
> + * after the server restart. We have the apply worker send those
> +statistics
> + * to the stats collector at prepare time and the startup process
> +restore
> + * those at restart if necessary.
> + */
> +static HTAB *subWorkerPreparedXactSizeHash = NULL;
> +
> +/*
Fixed. Also, its name was too long
when aligned with other PgStat_StatDBEntry memebers.
Thus I renamed it as subworkers_preparedsizes.

This depends on v21 in [1]

[1] - https://www.postgresql.org/message-id/CAD21AoAkd4YSoQUUFfpcrYOtkPRbninaw3sD0qc77nLW6Q89gg%40mail.gmail.com

Best Regards,
Takamichi Osumi

Attachment Content-Type Size
v10-0001-Rename-existing-columns-of-pg_stat_subscription_.patch application/octet-stream 15.4 KB
v10-0002-Extend-pg_stat_subscription_workers-to-include-g.patch application/octet-stream 53.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2021-11-15 09:27:55 Re: Write visibility map during CLUSTER/VACUUM FULL
Previous Message Dilip Kumar 2021-11-15 09:24:15 Re: Add connection active, idle time to pg_stat_activity