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(dot)janes(at)gmail(dot)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