Re: Race condition in SyncRepGetSyncStandbysPriority

Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> writes:
> On 2020/04/14 22:52, Tom Lane wrote:
>> *Yes it does*. The existing code can deliver entirely broken results
>> if some walsender exits between where we examine the priorities and
>> where we fetch the WAL pointers.

> IMO that the broken results can be delivered when walsender marked
> as sync exits *and* new walsender comes at that moment. If this new
> walsender uses the WalSnd slot that the exited walsender used,
> SyncRepGetOldestSyncRecPtr() wronly calculates the oldest lsn based
> on this new walsender (i.e., different walsender from one marked as sync).

Right, exactly, sorry that I was not more specific.

> BTW, since the patch changes the API of SyncRepGetSyncStandbys(),
> it should not be back-patched to avoid ABI break. Right?

Anything that is using that is just as broken as the core code is, for the
same reasons, so I don't have a problem with changing its API. Maybe we
should rename it while we're at it, just to make it clear that we are
breaking any external callers. (If there are any, which seems somewhat
unlikely.)

The only concession to ABI that I had in mind was to not re-order
the fields of WalSnd in the back branches.

regards, tom lane