On Thu, Nov 14, 2024 at 10:16 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Nov 14, 2024 at 7:08 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > Sure. I've attached the updated patch. I just added the commit message. > > > > @@ -1815,6 +1818,8 @@ LogicalIncreaseRestartDecodingForSlot(XLogRecPtr > current_lsn, XLogRecPtr restart > confirmed_flush = slot->data.confirmed_flush; > SpinLockRelease(&slot->mutex); > > + spin_released = true; > + > elog(DEBUG1, "failed to increase restart lsn: proposed %X/%X, after > %X/%X, current candidate %X/%X, current after %X/%X, flushed up to > %X/%X", > LSN_FORMAT_ARGS(restart_lsn), > LSN_FORMAT_ARGS(current_lsn), > @@ -1823,6 +1828,9 @@ LogicalIncreaseRestartDecodingForSlot(XLogRecPtr > current_lsn, XLogRecPtr restart > LSN_FORMAT_ARGS(confirmed_flush)); > } > > + if (!spin_released) > + SpinLockRelease(&slot->mutex); > > This coding pattern looks odd to me. We can consider releasing > spinlock in the other two if/else if checks. I understand it is a > matter of individual preference, so, if you and or others prefer the > current way, that is also fine with me. Other than this, the patch > looks good to me.
Indeed, I prefer your idea. I"ve attached the updated patch. I'll push it early next week unless there are further comments. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
v2-0001-Fix-a-possibility-of-logical-replication-slot-s-r.patch
Description: Binary data
