On Tue, Jul 7, 2020 at 7:07 AM Masahiko Sawada <masahiko.saw...@2ndquadrant.com> wrote: > > On Mon, 6 Jul 2020 at 20:45, Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > Second, as long as the unique identifier is the slot name there is no > > > convenient way to distinguish between the same name old and new > > > replication slots, so the backend process or wal sender process sends > > > a message to the stats collector to drop the replication slot at > > > ReplicationSlotDropPtr(). This strategy differs from what we do for > > > table, index, and function statistics. It might not be a problem but > > > I’m thinking a better way. > > > > > > > Can we rely on message ordering in the transmission mechanism (UDP) > > for stats? The wiki suggests [1] we can't. If so, then this might > > not work. > > Yeah, I'm also concerned about this. Another idea would be to have > another unique identifier to distinguish old and new replication slots > with the same name. For example, creation timestamp. And then we > reclaim the stats of unused slots later like table and function > statistics. >
So, we need to have 'creation timestamp' as persistent data for slots to achieve this? I am not sure of adding creation_time as a parameter to identify for this case because users can change timings on systems so it might not be a bullet-proof method but I agree that it can work in general. > On the other hand, if the ordering were to be reversed, we would miss > that stats but the next stat reporting would create the new entry. If > the problem is unlikely to happen in common case we can live with > that. > Yeah, that is a valid point and I think otherwise also some UDP packets can be lost so maybe we don't need to worry too much about this. I guess we can add a comment in the code for such a case. > > > > > Aside from the above, this patch will change the most of the changes > > > introduced by commit 9290ad198b1 and introduce new code much. I’m > > > concerned whether such changes are acceptable at the time of beta 2. > > > > > > > I think it depends on the final patch. My initial thought was that we > > should do this for PG14 but if you are suggesting removing the changes > > done by commit 9290ad198b1 then we need to think over it. I could > > think of below options: > > a. Revert 9290ad198b1 and introduce stats for spilling in PG14. We > > were anyway having spilling without any work in PG13 but didn’t have > > stats. > > b. Try to get your patch in PG13 if we can, otherwise, revert the > > feature 9290ad198b1. > > c. Get whatever we have in commit 9290ad198b1 for PG13 and > > additionally have what we are discussing here for PG14. This means > > that spilled stats at slot level will be available in PG14 via > > pg_stat_replication_slots and for individual WAL senders it will be > > available via pg_stat_replication both in PG13 and PG14. Even if we > > can get your patch in PG13, we can still keep those in > > pg_stat_replication. > > d. Get whatever we have in commit 9290ad198b1 for PG13 and change it > > for PG14. I don't think this will be a popular approach. > > I was thinking option (a) or (b). I'm inclined to option (a) since the > PoC patch added a certain amount of new codes. I agree with you that > it depends on the final patch. > Magnus, Tomas, others, do you have any suggestions on the above options or let us know if you have any other option in mind? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
