On Wed, Apr 10, 2024 at 11:09 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > No, I think you are misreading it, because the assignment is += not =. > The present coding is > > /* increase delay by a random fraction between 1X and 2X */ > status->cur_delay += (int) (status->cur_delay * > pg_prng_double(&pg_global_prng_state) + > 0.5); > > which looks fine to me. The +0.5 is so that the conversion to integer > rounds rather than truncating.
Oh, yeah ... right. But then why does the comment say that it's increasing the delay between a random fraction between 1X and 2X? Isn't this a random fraction between 0X and 1X? Or am I still full of garbage? > In any case, I concur with Andres: if this behavior is anywhere near > critical then the right fix is to not be using spinlocks. It would certainly be interesting to know which spinlocks were at issue, here. But it also seems to me that it's reasonable to be unhappy about the possibility of this increasing cur_delay by exactly 0. Storms of spinlock contention can and do happen on real-world production servers, and trying to guess which spinlocks might be affected before it happens is difficult. Until we fully eliminate all spinlocks from our code base, the theoretical possibility of such storms remains, and if making sure that the increment here is >0 mitigates that, then I think we should. Mind you, I don't think that the original poster has necessarily provided convincing or complete evidence to justify a change here, but I find it odd that you and Andres seem to want to dismiss it out of hand. -- Robert Haas EDB: http://www.enterprisedb.com
