Fixing transaction_timed_out errors with long running reads.

[apkar]

This patch attempts to fix transaction_timed_out errors in read path

• Remove wait() in choose-when block of doNonIsolatedRO, instead added another when block to represent the same logic.
• Running delay, that controls transaction split logic, at a higher priority.

[dongxinEric]

LGTM

[alexmiller-apple]

What is the priority at which this stream is yielding results?

[apkar]

Doc Layer code that is sending documents to this stream is not setting any priority explicitly. Source of the data is FDB flow bindings. Flow bindings set the priority to TaskDefaultOnMainThread (7500) and delay default priority is TaskDefaultDelay (7010). I tried this with TaskProxyGRVTimer (8510) instead of max priority, that too seems to be fixing the issue. Shall I go with it?

[alexmiller-apple]

Would it be equivalent and simpler to just do Void _ = wait( outerLock->take() || timeout ); instead of splitting this into two functions?

[apkar]

If I am thinking right, it would look like this

loop choose {
	when(state Reference<ScanReturnedContext> doc = waitNext(docs)) {
	// throws end_of_stream when totally finished
		Void _ = wait(outerLock->take() || timeout);
		if (!timeout.isReady()) {
                       .....
		}
	}
	when(Void _ = wait(timeout)) { break; }
}

// squeeze remaining documents from the stream

// checkpoint planner

This will need special handling with !timeout.isReady().

And due to the way query planner checkpoint scheme works, it's important there are no documents pending in document stream when the checkpoint is being called, that would be below this loop. I guess I can have extra code before the checkpoint to squeeze the documents left in the stream.

If so, probably having 3 when blocks probably clearer code.

[alexmiller-apple]

Do you not need to innerLock->release(); here also?

[apkar]

Because of checkpoint at line 730, innerLock is not valid anymore, no one is waiting on it.

[alexmiller-apple]

You're taking outerLock, but releasing innerLock, so what's making sure that you actually process documents as you add them?

[alexmiller-apple]

Do you need to pass a priority to take() as well?

[dongxinEric]

g_network->getCurrentTask() + 1 could still give you a task priority lower than TaskDefaultOnMainThread right?

[apkar]

Idea is current task is running at TaskDefaultMainThread as that’s the one setting the promise stream.