Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protocol command multiplexing. #2108

Open
wants to merge 64 commits into
base: integration
Choose a base branch
from

Conversation

dwsteele
Copy link
Member

@dwsteele dwsteele commented Jul 4, 2023

Previously it was not possible to read or write two files at the same time on the same remote because the protocol was entirely taken over by the read or write command. Multiple reads are required to make restores efficient when a list of bundled files is being read but blocks need to be retrieved from a separate file or a different part of the same file.

Improve that situation with sessions that allow related commands to be run with shared state. Also break the read/write into separate requests (rather than pushing all data at once) so they can be multiplexed.

The disadvantage for read/write is that they now require more back and forth to transfer a file. This is mitigated by sending asynchronous read/write request to keep both server and client as busy as possible. Reads that can fit into a single buffer are optimized to transfer in a single command. Reads that transfer the entire file can also skip the close command since it is implicit on end-of-file.

These changes allow the protocol to be simplified to provide one response per command, which makes the data end message obsolete. Any data sent for the command is now added to the parameters so no data needs to be sent separately to the server outside the command parameters.

Also update the Db protocol to use the new sessions. Previously this code had tracked its own sessions.

@dwsteele dwsteele self-assigned this Jul 4, 2023
@dwsteele dwsteele modified the milestones: 2.51, 2.52 Mar 25, 2024
@dwsteele
Copy link
Member Author

@sfrost This is ready for review again. We probably need some new documentation as well, but lets have a look first to make sure there are no major architectural changes needed.

# Conflicts:
#	src/command/restore/restore.c
#	src/command/verify/verify.c
#	src/config/protocol.c
#	src/db/db.c
#	src/db/protocol.c
#	src/postgres/client.c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant