• Tom Lane's avatar
    Be more predictable about reporting "lock timeout" vs "statement timeout". · 9dd4178c
    Tom Lane authored
    If both timeout indicators are set when we arrive at ProcessInterrupts,
    we've historically just reported "lock timeout".  However, some buildfarm
    members have been observed to fail isolationtester's timeouts test by
    reporting "lock timeout" when the statement timeout was expected to fire
    first.  The cause seems to be that the process is allowed to sleep longer
    than expected (probably due to heavy machine load) so that the lock
    timeout happens before we reach the point of reporting the error, and
    then this arbitrary tiebreak rule does the wrong thing.  We can improve
    matters by comparing the scheduled timeout times to decide which error
    to report.
    
    I had originally proposed greatly reducing the 1-second window between
    the two timeouts in the test cases.  On reflection that is a bad idea,
    at least for the case where the lock timeout is expected to fire first,
    because that would assume that it takes negligible time to get from
    statement start to the beginning of the lock wait.  Thus, this patch
    doesn't completely remove the risk of test failures on slow machines.
    Empirically, however, the case this handles is the one we are seeing
    in the buildfarm.  The explanation may be that the other case requires
    the scheduler to take the CPU away from a busy process, whereas the
    case fixed here only requires the scheduler to not give the CPU back
    right away to a process that has been woken from a multi-second sleep
    (and, perhaps, has been swapped out meanwhile).
    
    Back-patch to 9.3 where the isolationtester timeouts test was added.
    
    Discussion: <8693.1464314819@sss.pgh.pa.us>
    9dd4178c
postgres.c 118 KB