On Wed, Nov 10, 2021 at 3:43 PM vignesh C <vignes...@gmail.com> wrote: > > On Tue, Nov 9, 2021 at 5:05 PM osumi.takami...@fujitsu.com > <osumi.takami...@fujitsu.com> wrote: > > Yes. I've rebased and updated the patch, paying attention to this point. > > Attached the updated version. > > Thanks for the updated patch, few comments: > 6) Few places you have used strlcpy and few places you have used > memcpy, you can keep it consistent: > + msg.m_command = command; > + strlcpy(msg.m_gid, gid, sizeof(msg.m_gid)); > + msg.m_xact_bytes = xact_size; > > + key.subid = subid; > + memcpy(key.gid, gid, sizeof(key.gid)); > + action = (create ? HASH_ENTER : HASH_FIND);
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; 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; + +/* Regards, Vignesh
