Re: Replication slot stats misgivings

On Tue, Apr 20, 2021 at 7:54 PM Masahiko Sawada <sawada.mshk@gmail.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