On Fri, Jun 28, 2013 at 2:46 AM, Heikki Linnakangas <hlinnakan...@vmware.com > wrote:
> On 27.06.2013 17:30, Robert Haas wrote: > >> On Wed, Jun 26, 2013 at 5:49 PM, Jeff Janes<jeff.ja...@gmail.com> wrote: >> >>> If we think the patch has a risk of introducing subtle errors, then it >>> >>> probably can't be justified based on the small performance gains you saw. >>> >>> But if we think it has little risk, then I think it is justified simply >>> based on cleaner code, and less confusion for people who are tracing a >>> problematic process. If we want it to do a random escalation, it should >>> probably look like a random escalation to the interested observer. >>> >> >> I think it has little risk. If it turns out to be worse for >> performance, we can always revert it, but I expect it'll be better or >> a wash, and the results so far seem to bear that out. Another >> interesting question is whether we should fool with the actual values >> for minimum and maximum delays, but that's a separate and much harder >> question, so I think we should just do this for now and call it good. >> > > My thoughts exactly. I wanted to see if David Gould would like to do some > testing with it, but there's realy no need to hold off committing for that, > I'm not expecting any surprises there. Committed. > Thanks. > > Jeff, in the patch you changed the datatype of cur_delay variable from int > to long. AFAICS that makes no difference, so I kept it as int. Let me know > if there was a reason for that change. I think my thought process at the time was that since the old code multiplied by "1000L", not "1000" in the expression argument to pg_usleep, I wanted to keep the spirit of the L in place. It seemed safer than trying to prove to myself that it was not necessary. If int suffices, should the L come out of the defines for MIN_DELAY_USEC and MAX_DELAY_USEC ? Cheers, Jeff
