Re: One-off failure in "cluster" test

Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
> Ahh, I see what's happening. You don't need a concurrent process
> scanning *your* table for scan order to be nondeterministic. The
> preceding CLUSTER command can leave the start block anywhere if its
> call to ss_report_location() fails to acquire SyncScanLock
> conditionally. So I think we just need to disable that for this test,
> like in the attached.

Hmm. I'm not terribly thrilled about band-aiding one unstable test
case at a time.

heapgettup makes a point of ensuring that its scan end position
gets reported:

page++;
if (page >= scan->rs_nblocks)
page = 0;
finished = (page == scan->rs_startblock) ||
(scan->rs_numblocks != InvalidBlockNumber ? --scan->rs_numblocks == 0 : false);

/*
* Report our new scan position for synchronization purposes. We
* don't do that when moving backwards, however. That would just
* mess up any other forward-moving scanners.
*
* Note: we do this before checking for end of scan so that the
* final state of the position hint is back at the start of the
* rel. That's not strictly necessary, but otherwise when you run
* the same query multiple times the starting position would shift
* a little bit backwards on every invocation, which is confusing.
* We don't guarantee any specific ordering in general, though.
*/
if (scan->rs_base.rs_flags & SO_ALLOW_SYNC)
ss_report_location(scan->rs_base.rs_rd, page);

Seems like the conditional LWLockAcquire is pissing away that attempt
at stability. Maybe we should adjust things so that the final
location report isn't done conditionally.

regards, tom lane