Re: START_REPLICATION SLOT causing a crash in an assert build - Mailing list pgsql-hackers

Hi,

I wonder if the correct fix here wouldn't be to move the slotname out of
PgStat_StatReplSlotEntry?


On 2022-09-16 14:37:17 +0900, Kyotaro Horiguchi wrote:
> diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
> index 6224c498c2..ed3f3af4d9 100644
> --- a/src/backend/utils/activity/pgstat.c
> +++ b/src/backend/utils/activity/pgstat.c
> @@ -263,6 +263,8 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
>          .shared_size = sizeof(PgStatShared_Database),
>          .shared_data_off = offsetof(PgStatShared_Database, stats),
>          .shared_data_len = sizeof(((PgStatShared_Database *) 0)->stats),
> +        .reset_off = 0,
> +        .reset_len = sizeof(((PgStatShared_Database *) 0)->stats),
>          .pending_size = sizeof(PgStat_StatDBEntry),
>  
>          .flush_pending_cb = pgstat_database_flush_cb,
> @@ -277,6 +279,8 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
>          .shared_size = sizeof(PgStatShared_Relation),
>          .shared_data_off = offsetof(PgStatShared_Relation, stats),
>          .shared_data_len = sizeof(((PgStatShared_Relation *) 0)->stats),
> +        .reset_off = 0,
> +        .reset_len = sizeof(((PgStatShared_Relation *) 0)->stats),
>          .pending_size = sizeof(PgStat_TableStatus),
>  
>          .flush_pending_cb = pgstat_relation_flush_cb,
> @@ -291,6 +295,8 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
>          .shared_size = sizeof(PgStatShared_Function),
>          .shared_data_off = offsetof(PgStatShared_Function, stats),
>          .shared_data_len = sizeof(((PgStatShared_Function *) 0)->stats),
> +        .reset_off = 0,
> +        .reset_len = sizeof(((PgStatShared_Function *) 0)->stats),
>          .pending_size = sizeof(PgStat_BackendFunctionEntry),
>  
>          .flush_pending_cb = pgstat_function_flush_cb,
> @@ -307,6 +313,10 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
>          .shared_size = sizeof(PgStatShared_ReplSlot),
>          .shared_data_off = offsetof(PgStatShared_ReplSlot, stats),
>          .shared_data_len = sizeof(((PgStatShared_ReplSlot *) 0)->stats),
> +        /* reset doesn't wipe off slot name */
> +        .reset_off = offsetof(PgStat_StatReplSlotEntry, spill_txns),
> +        .reset_len = sizeof(((PgStatShared_ReplSlot *) 0)->stats),
> +        offsetof(PgStat_StatReplSlotEntry, spill_txns),

I'm confused what this offsetof does here? It's not even assigned to a
specific field? Am I missing something?

Also, wouldn't we need to subtract something of the size?


> diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
> index ac98918688..09a8c3873c 100644
> --- a/src/backend/utils/activity/pgstat_shmem.c
> +++ b/src/backend/utils/activity/pgstat_shmem.c
> @@ -915,8 +915,9 @@ shared_stat_reset_contents(PgStat_Kind kind, PgStatShared_Common *header,
>  {
>      const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
>  
> -    memset(pgstat_get_entry_data(kind, header), 0,
> -           pgstat_get_entry_len(kind));
> +    memset((char *)pgstat_get_entry_data(kind, header) +
> +           kind_info->reset_off, 0,
> +           kind_info->reset_len);
>  
>      if (kind_info->reset_timestamp_cb)
>          kind_info->reset_timestamp_cb(header, ts);

This likely doesn't quite conform to what pgindent wants...

Greetings,

Andres Freund