On 9/13/21, 11:06 PM, "Amul Sul" <sulamul@gmail.com> wrote:
> The patch is straightforward but the only concern is that in
> StartupXLOG(), SharedRecoveryState now gets updated only with spin
> lock; earlier it also had ControlFileLock in addition to that. AFAICU,
> I don't see any problem there, since until the startup process exists
> other backends could not connect and write a WAL record.
It looks like ebdf5bf intentionally made sure that we hold
ControlFileLock while updating SharedRecoveryInProgress (now
SharedRecoveryState after 4e87c48). The thread for this change [0]
has some additional details.
As far as the patch goes, I'm not sure why SetControlFileDBState()
needs to be exported, and TBH I don't know if this change is really a
worthwhile improvement. ISTM the main benefit is that it could help
avoid cases where we update the state but not the time. However,
there's still nothing preventing that, and I don't get the idea that
it was really a big problem to begin with.
Nathan
[0] https://postgr.es/m/CAB7nPqTS5J3-G_zTow0Kc5oqZn877RDDN1Mfcqm2PscAS7FnAw%40mail.gmail.com