Re: refactoring basebackup.c

> On Fri, Nov 05, 2021 at 11:50:01AM -0400, Robert Haas wrote:
> On Tue, Nov 2, 2021 at 10:32 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > Meanwhile, I think it's probably OK for me to go ahead and commit
> > 0001-0003 from my patches at this point, since it seems we have pretty
> > good evidence that the abstraction basically works, and there doesn't
> > seem to be any value in holding off and maybe having to do a bunch
> > more rebasing.
>
> I went ahead and committed 0001 and 0002, but got nervous about
> proceeding with 0003.

Hi,

I'm observing a strange issue which I can only relate to bef47ff85d
where bbsink abstraction was introduced. The problem is about failing
assertion when doing:

DETAIL: Failed process was running: BASE_BACKUP ( LABEL 'pg_basebackup base backup', PROGRESS, WAIT 0, MAX_RATE 102400, MANIFEST 'yes')

Walsender tries to send a backup manifest, but crashes on the trottling sink:

#2 0x0000560857b551af in ExceptionalCondition (conditionName=0x560857d15d27 "sink->bbs_next != NULL", errorType=0x560857d15c23 "FailedAssertion", fileName=0x560857d15d15 "basebackup_sink.c", lineNumber=91) at assert.c:69
#3 0x0000560857918a94 in bbsink_forward_manifest_contents (sink=0x5608593f73f8, len=32768) at basebackup_sink.c:91
#4 0x0000560857918d68 in bbsink_throttle_manifest_contents (sink=0x5608593f7450, len=32768) at basebackup_throttle.c:125
#5 0x00005608579186d0 in bbsink_manifest_contents (sink=0x5608593f7450, len=32768) at ../../../src/include/replication/basebackup_sink.h:240
#6 0x0000560857918b1b in bbsink_forward_manifest_contents (sink=0x5608593f74e8, len=32768) at basebackup_sink.c:94
#7 0x0000560857911edc in bbsink_manifest_contents (sink=0x5608593f74e8, len=32768) at ../../../src/include/replication/basebackup_sink.h:240
#8 0x00005608579129f6 in SendBackupManifest (manifest=0x7ffdaea9d120, sink=0x5608593f74e8) at backup_manifest.c:373

Looking at the similar bbsink_throttle_archive_contents it's not clear
why comments for both functions (archive and manifest throttling) say
"pass archive contents to next sink", but only bbsink_throttle_manifest_contents
does pass bbs_next into the bbsink_forward_manifest_contents. Is it
supposed to be like that? Passing the same sink object instead the next
one into bbsink_forward_manifest_contents seems to solve the problem in
this case.