-
Notifications
You must be signed in to change notification settings - Fork 203
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
dwsteele
wants to merge
64
commits into
pgbackrest:integration
Choose a base branch
from
dwsteele:dev-proto-multi
base: integration
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains 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
@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
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.
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.