On Sun, Nov 10, 2019 at 10:43:33AM +0530, Amit Kapila wrote: > On Sun, Nov 10, 2019 at 5:51 AM Jeff Janes <jeff.ja...@gmail.com> wrote: >> in src/backend/replication/walsender.c, there is the section >> quoted below. It looks like nothing interesting happens between >> the GetFlushRecPtr just before the loop starts, and the one inside >> the loop the first time through the loop. If we want to avoid >> doing CHECK_FOR_INTERRUPTS(); etc. needlessly, then we should >> check the result of GetFlushRecPtr and return early if it is >> sufficiently advanced--before entering the loop. If we don't >> care, then what is the point of updating it twice with no >> meaningful action >in between? We could just get rid of the >> section just before the loop starts. > > +1. I also think we should do one of the two things suggested by you. > I would prefer earlier as it can save us some processing in some cases > when the WAL is flushed in the meantime by WALWriter.
So your suggestion would be to call GetFlushRecPtr() before the first check on RecentFlushPtr and before entering the loop? It seems to me that we don't want to do that to avoid any unnecessary spin lock contention if the flush position is sufficiently advanced. Or are you just suggesting to move the first check on RecentFlushPtr within the loop just after resetting the latch but before checking for interrupts? If we were to do some cleanup here, I would just remove the first update of RecentFlushPtr before the loop as per the attached, which is the second suggestion from Jeff. Any thoughts? -- Michael
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 7f5671504f..f1a3f777f3 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -1304,12 +1304,6 @@ WalSndWaitForWal(XLogRecPtr loc) loc <= RecentFlushPtr) return RecentFlushPtr; - /* Get a more recent flush pointer. */ - if (!RecoveryInProgress()) - RecentFlushPtr = GetFlushRecPtr(); - else - RecentFlushPtr = GetXLogReplayRecPtr(NULL); - for (;;) { long sleeptime;
signature.asc
Description: PGP signature