On Sun, 19 Apr 2020 at 01:00, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Masahiko Sawada <masahiko.saw...@2ndquadrant.com> writes: > > On Sat, 18 Apr 2020 at 00:31, Tom Lane <t...@sss.pgh.pa.us> wrote: > >> + /* Quick out if not even configured to be synchronous */ > >> + if (SyncRepConfig == NULL) > >> + return false; > > > I felt strange a bit that we do the above check in > > SyncRepGetSyncRecPtr() because SyncRepReleaseWaiters() which is the > > only caller says the following before calling it: > > Notice there was such a test in SyncRepGetSyncRecPtr already --- I just > moved it to be before doing some work instead of after. > > > Can we either change it to an assertion, move it to before acquiring > > SyncRepLock in SyncRepReleaseWaiters or just remove it? > > I have no objection to that in principle, but it seems like it's a > change in SyncRepGetSyncRecPtr's API that is not necessary to fix > this bug. So I'd rather leave it to happen along with the larger > API changes (getting rid of am_sync) that are proposed for v14.
Agreed. Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
