Changing Sync Behavior

[1st1]

This will likely be merged into the ports RFC.

Preface

As of EdgeDB 1.0 Alpha 6, its binary protocol has the [SYNC] message, which mimics the semantics of the [SYNC] message in PostgreSQL binary protocol.

Here's an excerpt from the PostgreSQL documentation:

At completion of each series of extended-query messages, the frontend should issue a Sync message. This parameterless message causes the backend to close the current transaction if it's not inside a BEGIN/COMMIT transaction block ("close" meaning to commit if no error, or roll back if error). Then a ReadyForQuery response is issued. The purpose of Sync is to provide a resynchronization point for error recovery. When an error is detected while processing any extended-query message, the backend issues ErrorResponse, then reads and discards messages until a Sync is reached, then issues ReadyForQuery and returns to normal message processing. (But note that no skipping occurs if an error is detected while processing Sync — this ensures that there is one and only one ReadyForQuery sent for each Sync.)

Basically, any command after the last [SYNC] opens a new implicit transaction. Any commands issued after that will be run in that transaction, until another [SYNC] message commits it. For example, let's consider the following sequence of client protocol messages:

1. [...]
2. [SYNC], and then after some time:
3. [EXECUTE query="INSERT Foo ..."]
4. [EXECUTE query="START TRANSACTION"]
5. [EXECUTE query="INSERT Bar ..."]
6. [EXECUTE query="ROLLBACK"]
7. [SYNC]

Perhaps surprisingly, both INSERT Foo ... and INSERT Bar ... statements will be rollbacked. This happens because at (3), the PostgreSQL server would start a new transaction and run (3) in it. Later, (4) would be basically meaningless (unless it specifies isolation level), as the transaction has already been started. Then (6) would cancel the transaction as well as the effects of (3) and (5).

The design of Sync makes sense for PostgreSQL protocol. It allows fine-grained control over the DB connection state. In EdgeDB, especially with the new ports RFC soon to be implemented, [SYNC] poses some challenges:

• The implicit transactions semantics means that EdgeDB client connections will have to be pinned to backend PostgreSQL connections until the implicit transaction is closed. Consider the very basic example of a client sending [PREPARE] and waiting for the server to reply; once the response is received, the client would send [EXECUTE] and then [SYNC]. The implicit transaction would be open between receiving [PREPARE] and the final [SYNC]. Therefore, we will have to reserve a backend PostgreSQL connection exclusively for this EdgeDB client.

• EdgeDB client libraries that fail to implement [SYNC] properly will quickly exhaust server backend connections.

Proposal

The proposal boils down to the following few adjustments:

• EdgeDB protocol will not support implicit transactions. Transactions will have to be open explicitly with EdgeQL.

• The [SYNC] message will stay and will be used by clients to:

  • Query the connection transaction state.
  • Synchronize protocol when an error happens. If an exception happens, EdgeDB server, just like PostgreSQL, will ignore all commands up until the client sends Sync.

• [PREPARE] will become lazy w.r.t. actually preparing a statement in PostgreSQL. With the new semantics, [PREPARE] will parse and compile the EdgeQL query, returning type descriptors (or an error) to the client and caching the compiled SQL. The SQL code will actually be prepared in PostgreSQL when EdgeDB receives an [EXECUTE] message.

• EdgeDB server will no longer always relay [SYNC] messages to PostgreSQL.

Backwards Compatibility

The existing drivers should continue working unless they rely on implicit transactions. To the best of my knowledge, our Python/JS/Rust drivers do not rely on them.

What if we want PostgreSQL semantics one day?

We can add a new protocol message to control transactions explicitly. Although it's currently unclear what kind of use cases would require that.

[1st1]

@edgedb/engineering

[elprans]

• The SQL code will actually be prepared in PostgreSQL when EdgeDB receives an [EXECUTE] message.

This part is necessary for proper backend connection pooling in any case, where each connection will maintain its own prepared statement cache.

EdgeDB will not open implicit transactions. Transactions will have to be open explicitly with EdgeQL.

• EdgeDB server will no longer always relay [SYNC] messages to PostgreSQL.

I'm having trouble reconciling these two points. In my mind, it's the opposite: we must make sure that, outside of an explicit EdgeQL transaction, we always send [SYNC] after each statement, even if the client didn't send [SYNC] (auto-commit mode). At the same time, EdgeDB [SYNC] should basically never trigger PostgreSQL [SYNC]. In other words, we remove all transactional semantics from EdgeDB [SYNC].

[1st1]

This part is necessary for proper backend connection pooling in any case, where each connection will maintain its own prepared statement cache.

Correct.

I'm having trouble reconciling these two points. In my mind, it's the opposite: we must make sure that, outside of an explicit EdgeQL transaction, we always send [SYNC] after each statement, even if the client didn't send [SYNC] (auto-commit mode). At the same time, EdgeDB [SYNC] should basically never trigger PostgreSQL [SYNC]. In other words, we remove all transactional semantics from EdgeDB [SYNC].

Yes. That's what I tried to convey with "relay" -- EdgeDB will on its own manage PostgreSQL SYNCs (the way you described). But the current assumption that an EdgeDB SYNC always translates to a PostgreSQL SYNC will no longer be true.

I'll edit the first bullet to make it less confusing.

[tailhook]

This sounds like if we're getting error we should send [sync] and wait for response. I think that sending [execute][sync] always may be more useful anyway. Note that error response can be on the hot path, in a case like this:

try:
    c.query("INSERT X {...}")
except IntegrityError:
    c.query("UPDATE X SET {...}")

(not every such use case can be covered by UNLESS CONFLICT)

[1st1]

I think that sending [execute][sync] always may be more useful anyway.

Yep, that will be the way we recommend people to use it as it's an easier way to program the protocol than only send [sync] on error. You could potentially send [execute][execute][sync] but I don't see any utility in that now.

[tailhook]

The problem with non starting implicit transaction on Prepare is: what if schema changed between Prepare and Execute, and query is still valid but descriptor is not? E.g. SELECT User { prop1 }, where prop1 was int32 and now is str.

[1st1]

To expand:

I mean in Postgres you can create a named prepared statement and it might expire any time and you'll need to re-prepare it. This is what we do in asyncpg in cases like this. Granted this isn't a problem with anonymous prepared statements in PostgreSQL because they exist in an implicit transaction, but we have some options there as well:

1. We can make this an auto-retriable error in our bindings.
2. We can re-prepare anonymous statements automatically on the server side 1 or 2 times if we see that the schema has changed right before [EXECUTE].

[tailhook]

We don't re-prepare statements in any current client. Is this something that should be a part of RFC1004? We should at least figure out whether that error class is specific enough and fits the hierarchy of retriable errors.

[tailhook]

And also RFC1004 retries only read-only statements and within retry block. But standalone modification queries should be re-prepared now too.

[1st1]

We don't re-prepare statements in any current client. Is this something that should be a part of RFC1004? We should at least figure out whether that error class is specific enough and fits the hierarchy of retriable errors.

+1

BTW, when I said

We can re-prepare anonymous statements automatically 1 or 2 times if we see that the schema has changed right before [EXECUTE].

I meant we can do that on the server side. I'll update that message to make that clear.

[tailhook]

I meant we can do that on the server side. I'll update that message to make that clear.

We can not in case descriptor changes (and query is still valid syntactically). Like in the example above where property type has changed.