Hi, I was looking at this code comment and wondered what it meant. AFAICT over time code has been moved around causing comments to lose their original context, so now it is hard to understand what they are saying.
~~~ After a 2017 patch [1] the code in walsender.c function logical_read_xlog_page() looked like this: /* make sure we have enough WAL available */ flushptr = WalSndWaitForWal(targetPagePtr + reqLen); /* fail if not (implies we are going to shut down) */ if (flushptr < targetPagePtr + reqLen) return -1; ~~~ The same code in HEAD now looks like this: /* * Make sure we have enough WAL available before retrieving the current * timeline. This is needed to determine am_cascading_walsender accurately * which is needed to determine the current timeline. */ flushptr = WalSndWaitForWal(targetPagePtr + reqLen); /* * Since logical decoding is also permitted on a standby server, we need * to check if the server is in recovery to decide how to get the current * timeline ID (so that it also cover the promotion or timeline change * cases). */ am_cascading_walsender = RecoveryInProgress(); if (am_cascading_walsender) GetXLogReplayRecPtr(&currTLI); else currTLI = GetWALInsertionTimeLine(); XLogReadDetermineTimeline(state, targetPagePtr, reqLen, currTLI); sendTimeLineIsHistoric = (state->currTLI != currTLI); sendTimeLine = state->currTLI; sendTimeLineValidUpto = state->currTLIValidUntil; sendTimeLineNextTLI = state->nextTLI; /* fail if not (implies we are going to shut down) */ if (flushptr < targetPagePtr + reqLen) return -1; ~~~ Notice how the "fail if not" comment has become distantly separated from the flushptr assignment it was once adjacent to, so that comment hardly makes sense anymore -- e.g. "fail if not" WHAT? Perhaps the comment should say something like it used to: /* Fail if there is not enough WAL available. This can happen during shutdown. */ ====== [1] https://github.com/postgres/postgres/commit/fca85f8ef157d4d58dea1fdc8e1f1f957b74ee78 Kind Regards, Peter Smith. Fujitsu Australia
