Hi Ashutosh, Thanks for the patch. LGTM overall.
On Mon, Nov 2, 2026 at 6:12 PM Ashutosh Bapat <[email protected]> wrote: > > Should we ERROR if it is called more than once? That might need some > > special handling for the restart_after_crash case. > > > > With this change, a second call to ShmemGetRequestedSize() during the > same shared-memory initialization cycle fails > Assert(shmem_request_state == SRS_REQUESTING) in assertion-enabled > builds. This also detects repeated calls through CalculateShmemSize(). > I think an assertion is sufficient here because this is an internal > lifecycle invariant, consistent with the other state-transition > assertions. Attempts to register requests after sizing are still > rejected by the existing runtime checks. > > For restart_after_crash, ResetShmemAllocator() resets the state to > SRS_INITIAL. The request callbacks then start a fresh request phase, > allowing the size to be calculated again for the new segment. > Erroring out can help to stop the misake where it occurs. A build without assert could accept the bad call, then fail later somewhere else. It might make the problem a bit hard to understand. That said, assert seems ok here since we haven't found code can make that bad call. A nit - the new state in code is SRS_SIZED, but the commit message says SRS_SIZING. -- Regards, Xuneng Zhou HighGo Software Co., Ltd.
