Dear Vitaly, > > Per my understanding, this happened because there is a lag that restart_lsn > > of > > the slot is set, and it is protected by the system. Your idea is to ensure > > the > > restart_lsn is protected by the system before obtaining on-memory LSN, > > right? > > Not sure what you mean by on-memory LSN, but, the issue happens because we > have > a lag between restart_lsn assignment and update of > XLogCtl->replicationSlotsMinLSN which is used to protect the WAL.
Sorry I should say "before obtaining replicationSlotMinLSN". > Yes, I > propose > to ensure that the protection happens when we assign restart_lsn. It seems to > be > wrong that we invalidate slots by its restart_lsn but protect the wal for > slots using XLogCtl->replicationSlotsMinLSN. Seems valid. There is another corner case that another restart_lsn can be set in-between, but they have larger LSN than RedoRecPtr, right? > Below I tried to write some summary and propose the patch which fixes the > problem. Sorry but it is too long to understand properly for me :-(. > There is one subtle thing. Once, the operation of restart_lsn assignment is > not > an atomic, the following scenario may happen theoretically: > 1. Read GetRedoRecPtr() in the backend (ReplicationSlotReserveWal) > 2. Assign a new redo LSN in the checkpointer > 3. Call ReplicationSlotsComputeRequiredLSN() in the checkpointer > 3. Assign the old redo LSN to restart_lsn > > In this scenario, the restart_lsn will point to a previous redo LSN and it > will > be not protected by the new redo LSN. This scenario is unlikely, but it can > happen theoretically. I have no ideas how to deal with it, except of assigning > restart_lsn under XLogCtl->info_lck lock to avoid concurrent modification of > XLogCtl->RecoRecPtr until it is assigned to restart_lsn of a creating slot. Oh, your point is there is another race condition, right? Do you have the reproducer for it? Best regards, Hayato Kuroda FUJITSU LIMITED
