Thanks for the feedbacks!
To be honest, I'm fine with v3, since it only increases the sleep time
after 5000 loop iterations, which has negligible performance impact.
But if these functions aren't intended to be used during recovery and
the loop shouldn't reach that many iterations, I'm also okay with
the v4 approach of introducing a separate function specifically for recovery.
But this amakes me wonder if we should add something like
Assert(!RecoveryInProgress()) to those two functions, to prevent them
from being used during recovery in the future.
> but it's hard to find a proper place for it to fit well: lmgr.c is for locks, while standby.c or procarray.c are not that ideal either. Therefore, I placed the special handling for standby in SnapBuildWaitSnapshot.
Since the purpose and logic of the new function are similar to
XactLockTableWait(), I think it would be better to place it nearby
in lmgr.c, even though it doesn't handle a lock directly. That would
help keep the related logic together and improve readability.
Now I see two possible approaches here. One is to extend XactLockTableWait(), and the other is to introduce a new wait function specifically for standby. For the first option, adding standby-specific logic might not align well with the function’s name or its original design. If we go with a new function, we might need to consider what other scenarios it could be reused in. If this is the only place it would apply, whether it’s worth introducing a separate function just for this case.
Best regards,
Xuneng
