From: Tom Lane <tgl@sss.pgh.pa.us>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: masahiko.sawada@2ndquadrant.com
Cc: masao.fujii@oss.nttdata.com
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Race condition in SyncRepGetSyncStandbysPriority
Date: Wed, 15 Apr 2020 11:31:49 -0400
Message-ID: <22258.1586964709@sss.pgh.pa.us> (raw)
In-Reply-To: <20200415.113558.363461930251464527.horikyota.ntt@gmail.com>
References: <20200414.183538.344904915964423903.horikyota.ntt@gmail.com>
<15616.1586872362@sss.pgh.pa.us>
<13059.1586896360@sss.pgh.pa.us>
<20200415.113558.363461930251464527.horikyota.ntt@gmail.com>
Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> At Tue, 14 Apr 2020 16:32:40 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in
> + stby->is_sync_standby = true; /* might change below */
> I'm uneasy with that. In quorum mode all running standbys are marked
> as "sync" and that's bogus.
I don't follow that? The existing coding of SyncRepGetSyncStandbysQuorum
returns all the candidates in its list, so this is isomorphic to that.
Possibly a different name for the flag would be more suited?
> On the other hand sync_standbys is already sorted in priority order so I think we can get rid of the member by setting *am_sync as the follows.
> SyncRepGetSyncRecPtr:
> if (sync_standbys[i].is_me)
> {
> *am_sync = (i < SyncRepConfig->num_sync);
> break;
> }
I disagree with this, it will change the behavior in the quorum case.
In any case, a change like this will cause callers to know way more than
they ought to about the ordering of the array. In my mind, the fact that
SyncRepGetSyncStandbysPriority is sorting the array is an internal
implementation detail; I do not want it to be part of the API.
(Apropos to that, I realized from working on this patch that there's
another, completely undocumented assumption in the existing code, that
the integer list will be sorted by walsender index for equal priorities.
I don't like that either, and not just because it's undocumented.)
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, horikyota.ntt@gmail.com, masahiko.sawada@2ndquadrant.com, masao.fujii@oss.nttdata.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Race condition in SyncRepGetSyncStandbysPriority
In-Reply-To: <22258.1586964709@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox