Re: Replication slot stats misgivings

On Tue, Apr 20, 2021 at 7:54 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:

>
> I've attached the patch. In addition to the test Vignesh prepared, I
> added one test for the message for creating a slot that checks if the
> statistics are initialized after re-creating the same name slot.
> Please review it.

Overall the patch looks good to me. However, I have one question, I
did not understand the reason behind moving the below code from
"pgstat_reset_replslot_counter" to "pg_stat_reset_replication_slot"?

+ /*
+ * Check if the slot exists with the given name. It is possible that by
+ * the time this message is executed the slot is dropped but at least
+ * this check will ensure that the given name is for a valid slot.
+ */
+ slot = SearchNamedReplicationSlot(target, true);
+
+ if (!slot)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("replication slot \"%s\" does not exist",
+ target)));
+
+ /*
+ * Nothing to do for physical slots as we collect stats only for
+ * logical slots.
+ */
+ if (SlotIsPhysical(slot))
+ PG_RETURN_VOID();
+ }
+
pgstat_reset_replslot_counter(target);

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com