Optimize protocol round trips for commands and queries - #1356
Open
davidlar wants to merge 5 commits into
Open
Conversation
An exchange is a send->receive transition, which on a high-latency link dominates the time a statement takes. Needs no database.
A command never pages, so it has no reason to keep the portal open between Executes and can end the exchange in the same write. Issue typelevel#210 requires that Sync be sent, not that it be sent after the completion read.
Sync ends the implicit transaction and takes the portal with it, so outside an explicit transaction the Close is a round trip spent on a no-op. ReadyForQuery already reports which case applies.
execute(query)(args), unique and option fetch once, so they have no use for a portal that outlives the fetch. stream still pages and is unchanged.
cursor(args).use(_.fetch(Int.MaxValue)) is the obvious way to ask for all rows and costs four exchanges where one will do. Also lets Session.Impl route execute(query)(args) through it, removing two overrides.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We run a service whose database is in another country. At ~60 ms round-trip time the driver's message count is the cost of a small statement — execution is single-digit milliseconds, so wall-clock is
exchanges × RTT.Parse+DescribeandBind+Executeare each already pipelined behind oneFlush(#1059, #1061),but the exchanges between those groups, plus the trailing
Syncand portalClose, were separatewaits.
Exchanges per operation
execute(query)(args)unique/optionstreamtestOnly tests.simulation.ExchangeCountTestprints this table and asserts every number; it needs nodatabase.
Results
In production since 31 August — one day so far, but under heavy load against several databases
(Postgres 14 and 16) in different datacentres, on both the ~60 ms link and low-latency ones, with no
protocol errors or session problems. On a transaction that reads and then writes we see about 40%
lower latency, consistent with 4→2 exchanges for the reads and 3→2 for the writes at 60 ms. That is
the conservative case: inside a transaction the portal outlives
Sync, so theClosestays and eachstatement saves one round trip rather than two.
With latency injected locally, medians of three runs at 40 ms RTT: command 134→44 ms,
unique128→45,
option127→47,execute(query)(args)170→48, andstream— the control, whoseexchange count does not change — 128→129. At 100 ms the rows scale as
exchanges × RTTpredicts. On loopback the difference is within noise, so this buys little if your database is next door.What changed
Syncin the same write asBind+Execute. A command never pages, so it has noreason to hold the portal open between
Executes. Commands not completing until next query/command is run #210 requires thatSyncbe sent, not that itbe sent after the completion read.
Closeis skipped when the backend has already dropped it:Syncends the implicittransaction and takes the portal with it, and
ReadyForQueryalready reports which case applies.execute(query)(args),unique,option) get the same treatment — they fetchonce by construction, so no second
Executecan be stranded by ending the transaction.stream,cursorandpipeare untouched: paging needs the portal to outlive oneExecute, whichis why
Flushand notSyncis correct there.Points worth a close look
Flush/Syncin copy-in mode, so the pre-sentSyncis swallowed and that branch sends its own.inside its handling of
Executeand only then reads the next frontend message, so it reaches thepre-sent
Syncafterwards and repliesReadyForQuery, which must be consumed.CopyOutTestcorroborates it.
unique/optionask for 2 rows to tell whether more exist. When more do,the portal suspends rather than completing, and its
ReadyForQuerystill has to be read or thenext operation mistakes it for its own. That desynchronises the session rather than failing a test,
so the tests assert that a follow-up operation still succeeds.
Production covers the ordinary paths under concurrency, but not copy-out, and reaches (3) only when
unique/optionerrors on extra rows.Review
Five commits, each self-contained and green on its own — the first is only the measurement harness,
and 2–4 are independent protocol changes. Happy to split into separate PRs if you would prefer.
PreparedQuery.fetchAllis the one real API addition:cursor(args).use(_.fetch(Int.MaxValue))isthe obvious way to ask for all rows and costs four exchanges where one will do. Happy to drop that
commit if you would rather not grow the API; nothing else depends on it.