On 9/13/21, 11:06 PM, "Amul Sul" <sula...@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
