At Fri, 7 Oct 2022 12:14:40 +0900, Masahiko Sawada <sawada.m...@gmail.com> wrote in > > What about if we go the other direction - simply remove the name from the > > stats entry at all. I don't actually think we need it anymore. Unless I am > > missing something right now - entirely possible! - the danger that > > pgstat_acquire_replslot() mentions doesn't actually exist [anymore]. After a > > crash we throw away the old stats data and if a slot is dropped while shut > > down, we'll not load the slot data at startup.
The key point of this is this: + * XXX: I think there cannot actually be data from an older slot + * here. After a crash we throw away the old stats data and if a slot is + * dropped while shut down, we'll not load the slot data at startup. I think this is true. Assuming that we don't recreate or rename objects that have stats after writing out stats, we won't have stats for a different object with the same name. If we can rely on that fact, the existing check in pgstat_acquire_replslot() becomes useless. Thus we don't need to store object name in stats entry. I agree to that. > +1. I think it works. Since the replication slot index doesn't change > during server running we can fetch the name from > ReplicationSlotCtl->replication_slots. That access seems safe in a bit different aspect, too. Both checkpointer (and walsender) properly initialize ReplicationSlotCtl. > If we don't need the name in stats entry, pgstat_acquire_replslot() is > no longer necessary? I think so. The entry will be created at the first report. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
