Re: Spurious pgstat_drop_replslot() call
От | Bertrand Drouvot |
---|---|
Тема | Re: Spurious pgstat_drop_replslot() call |
Дата | |
Msg-id | Zeso6vvK+k/jq2+i@ip-10-97-1-34.eu-west-3.compute.internal обсуждение исходный текст |
Ответ на | Re: Spurious pgstat_drop_replslot() call (Michael Paquier <michael@paquier.xyz>) |
Ответы |
Re: Spurious pgstat_drop_replslot() call
|
Список | pgsql-hackers |
Hi, On Fri, Mar 08, 2024 at 07:55:39PM +0900, Michael Paquier wrote: > On Fri, Mar 08, 2024 at 10:19:11AM +0000, Bertrand Drouvot wrote: > > Indeed, it does not seem appropriate to remove stats during slot invalidation as > > one could still be interested to look at them. > > Yeah, my take is that this can still be interesting to know, at least > for debugging. This would limit the stats to be dropped when the slot > is dropped, and that looks like a sound idea. Thanks for looking at it! > > This spurious call has been introduced in be87200efd. I think that initially we > > designed to drop slots when a recovery conflict occurred during logical decoding > > on a standby. But we changed our mind to invalidate such a slot instead. > > > > The spurious call is probably due to the initial design but has not been removed. > > This is not a subject that has really been touched on the original > thread mentioned on the commit, so it is a bit hard to be sure. The > only comment is that a previous version of the patch did the stats > drop in the slot invalidation path at an incorrect location. The switch in the patch from "drop" to "invalidation" is in [1], see: " Given the precedent of max_slot_wal_keep_size, I think it's wrong to just drop the logical slots. Instead we should just mark them as invalid, like InvalidateObsoleteReplicationSlots(). Makes fully sense and done that way in the attached patch. “ But yeah, hard to be sure why this call is there, at least I don't remember... > > I don't think it's worth to add a test but can do if one feel the need. > > Well, that would not be complicated while being cheap, no? We have a > few paths in the 035 test where we know that a slot has been > invalidated so it is just a matter of querying once > pg_stat_replication_slot and see if some data is still there. We can not be 100% sure that the stats are up to date when the process holding the active slot is killed. So v2 attached adds a test where we ensure first that we have non empty stats before triggering the invalidation. [1]: https://www.postgresql.org/message-id/26c6f320-98f0-253c-f8b5-df1e7c1f6315%40amazon.com Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Вложения
В списке pgsql-hackers по дате отправления: