Fix pgstat_database.c to honor passed database OIDs - Mailing list pgsql-hackers

From Chao Li
Subject Fix pgstat_database.c to honor passed database OIDs
Date
Msg-id ABBD5026-506F-4006-A569-28F72C188693@gmail.com
Whole thread Raw
Responses Re: Fix pgstat_database.c to honor passed database OIDs
List pgsql-hackers
Hi,

In pgstat_database.c, pgstat_report_connect(), pgstat_report_disconnect(), and pgstat_reset_database_timestamp() all
takea dboid parameter, but currently ignore it and use MyDatabaseId instead. While that does not seem to break anything
today,it at least hurts readability. 

This patch changes those three functions to use the passed dboid.

For pgstat_report_connect() and pgstat_report_disconnect(), there is only one caller, and it passes MyDatabaseId, so
thischange should be safe. 

For pgstat_reset_database_timestamp(), in most paths dboid is also just MyDatabaseId. However, there is one path where
dboidcan be InvalidOid: 

1 - pg_stat_reset_single_table_counters may pass InvalidOid to pgstat_reset for a shared relation.
```
Datum
pg_stat_reset_single_table_counters(PG_FUNCTION_ARGS)
{
    Oid            taboid = PG_GETARG_OID(0);
    Oid            dboid = (IsSharedRelation(taboid) ? InvalidOid : MyDatabaseId);

    pgstat_reset(PGSTAT_KIND_RELATION, dboid, taboid);

    PG_RETURN_VOID();
}
```

2 - pgstat_reset only calls pgstat_reset_database_timestamp when kind_info->accessed_across_databases is false.
```
void
pgstat_reset(PgStat_Kind kind, Oid dboid, uint64 objid)
{
    const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
    TimestampTz ts = GetCurrentTimestamp();

    /* not needed atm, and doesn't make sense with the current signature */
    Assert(!pgstat_get_kind_info(kind)->fixed_amount);

    /* reset the "single counter" */
    pgstat_reset_entry(kind, dboid, objid, ts);

    if (!kind_info->accessed_across_databases)
        pgstat_reset_database_timestamp(dboid, ts);
}
```

3 - In this path, kind is PGSTAT_KIND_RELATION, and accessed_across_databases is false:
```
    [PGSTAT_KIND_RELATION] = {
        .name = "relation",

        .fixed_amount = false,
        .write_to_file = true,

        .shared_size = sizeof(PgStatShared_Relation),
        .shared_data_off = offsetof(PgStatShared_Relation, stats),
        .shared_data_len = sizeof(((PgStatShared_Relation *) 0)->stats),
        .pending_size = sizeof(PgStat_TableStatus),

        .flush_pending_cb = pgstat_relation_flush_cb,
        .delete_pending_cb = pgstat_relation_delete_pending_cb,
        .reset_timestamp_cb = pgstat_relation_reset_timestamp_cb,
    },
```

So in this case, pgstat_reset_database_timestamp() can receive InvalidOid as dboid. In the current code, because that
functionignores dboid and uses MyDatabaseId, calling pg_stat_reset_single_table_counters() on a shared relation can
incorrectlyreset the current database's stat_reset_timestamp. That behavior seems unintended, so this patch makes
pgstat_reset_database_timestamp()return immediately when dboid is InvalidOid. 

Please see the attached patch. “Make check-world” passed from my side.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/





Attachment

pgsql-hackers by date:

Previous
From: Richard Guo
Date:
Subject: Re: Bug: var_is_nonnullable() gives wrong results for old/new in RETURNING
Next
From: shveta malik
Date:
Subject: Re: Fix slotsync worker busy loop causing repeated log messages