Ok I have investigated this more. Explaining here as requested by @petermattis.

@andreimatei you may want to chime in since the main focus here ends up revolving around autoretries and API.

The issue to solve here is not that clients may be confused to receive an error after some data has been communicated already. It is actually valid in pgwire (and other sql wire protocols) to start returning results and then fail before the statement has completed.

The main issue I found is that the API between pgwire and Executor is currently a function that drives the entire SQL transaction state, the mapping between the SQL transaction state and KV transaction state, and the retry logic. To change this to a streaming interface is not simple. It amounts to an API change that will break the abstraction boundary and cause headaches with retries.

A naive idea, not to be considered seriously but related here for contrast with the other idea below, is to change the Executor API to return a generator of planNodes to pgwire, to be driven via Start/Next etc by pgwire. However if/when an error occurs or anything like a transaction boundary in the middle of the SQL syntax, the Executor must be informed of this accordingly by pgwire (during the progress of the stream of planNodes). This would amount to a complex bi-directional interaction between pgwire and Executor, ie too complex for what we want to maintain.

The more promising direction is to have pgwire pass a closure to Executor, like a "sender function" that the Executor can use to actively send the results towards pgwire during ExecStatements.

Now the problem with this is what to do with transaction retries. The main issue is that as soon as some data has flown on the wire from the executor to the client, a retry becomes impossible. IF we naively start to stream data as soon as a statement produces data, no implicit retry becomes ever possible again for out-of-txn SELECT or INSERT/UPDATE/UPSERT ... RETURNING. So the better solution, assuming we want to keep autoretries, would be to start accumulating results locally in the server, and only stream that back to the client as soon as some local capacity limit has been reached, and limit autoretry to those statements for which the results has fit entirely in the buffer (and have not been communicated back to the client yet).

Overall all this looks pretty doable with regards to functionality but there will be a lot of corner cases for testing, namely what happens with this buffering of rows in the presence of client errors and txn errors, or when there is a transaction boundary inside a single SQL string (like "... COMMIT; BEGIN ...").

sql: stream planNode.Next() results through pgwire without collecting all intermediate results in RAM #7775

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions