Horton

Re: Replication slot stats misgivings

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Date: 2021-04-20T09:59:16Z
Lists: pgsql-hackers

Attachments

On Tue, Apr 20, 2021 at 9:08 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> I've attached the new version patch that fixed the compilation error
> reported off-line by Amit.
>

I was thinking about whether we can someway avoid the below risk:
In case where the
+ * message for dropping the old slot gets lost and a slot with the same
+ * name is created, the stats will be accumulated into the old slots since
+ * we use the slot name as the key. In that case, user can reset the
+ * particular stats by pg_stat_reset_replication_slot().

What if we send a separate message for create slot such that the stats
collector will initialize the entries even if the previous drop
message is lost or came later? If we do that then if the drop message
is lost, the create with same name won't accumulate the stats and if
the drop came later, it will remove the newly created stats but
anyway, later stats from the same slot will again create the slot
entry in the hash table.

Also, I think we can include the test case prepared by Vignesh in the email [1].

Apart from the above, I have made few minor modifications in the attached patch.
(a) + if (slotent->stat_reset_timestamp == 0 || !slotent)
I don't understand why second part of check is required? By this time
slotent will anyway have some valid value.

(b) + slotent = (PgStat_StatReplSlotEntry *) hash_search(replSlotStats,
+    (void *) &name,
+    create_it ? HASH_ENTER : HASH_FIND,
+    &found);

It is better to use NameStr here.

(c) made various changes in comments and some other cosmetic changes.

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

-- 
With Regards,
Amit Kapila.

Commits

  1. Doc: Update logical decoding stats information.

  2. Fix tests for replication slots stats.

  3. Update replication statistics after every stream/spill.

  4. Fix the computation of slot stats for 'total_bytes'.

  5. Another try to fix the test case added by commit f5fc2f5b23.

  6. Use HTAB for replication slot statistics.

  7. Fix test case added by commit f5fc2f5b23.

  8. Add information of total data processed to replication slot stats.

  9. Use NameData datatype for slotname in stats.