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

Fix reader serialization for old clients. #3446

Merged
merged 2 commits into from Aug 15, 2022

Conversation

KiterLuc
Copy link
Contributor

Old clients might submit a query using the legacy reader when the server
would select the refactored ones. This causes au issue because the read
state gets deserialized by the client and used to determine query
completion. The server needs to serialize a read state that the client
will understand, so for now, if the client decides to use the legacy
reader, the server needs to process that query with the legacy reader.

Long term, we will want to serialize a member of the query to determine
completion, and the read state will be serialized as a blob not
deserialized by the client. That way, the server can decide to use
whatever reader it wants.


TYPE: IMPROVEMENT
DESC: Fix reader serialization for old clients.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #20085: Segfault in core caused by VCF.

Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to add a test if there's a straightforward way to do that. Otherwise LGTM.

@KiterLuc
Copy link
Contributor Author

@ihnorton I removed the backport to 2.11 as it would require to backport another, more elaborate PR that was merged in 2.12.

Old clients might submit a query using the legacy reader when the server
would select the refactored ones. This causes au issue because the read
state gets deserialized by the client and used to determine query
completion. The server needs to serialize a read state that the client
will understand, so for now, if the client decides to use the legacy
reader, the server needs to process that query with the legacy reader.

Long term, we will want to serialize a member of the query to determine
completion, and the read state will be serialized as a blob not
deserialized by the client. That way, the server can decide to use
whatever reader it wants.

---
TYPE: IMPROVEMENT
DESC: Fix reader serialization for old clients.
@KiterLuc KiterLuc force-pushed the lr/fix-serialization-old-clients/ch20085 branch from f8ffb9e to fc86605 Compare August 13, 2022 21:58
@ihnorton
Copy link
Member

Thanks for adding the test 👍

@ihnorton ihnorton merged commit d4a41b4 into dev Aug 15, 2022
@ihnorton ihnorton deleted the lr/fix-serialization-old-clients/ch20085 branch August 15, 2022 13:13
@github-actions
Copy link
Contributor

The backport to release-2.11 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release-2.11 release-2.11
# Navigate to the new working tree
cd .worktrees/backport-release-2.11
# Create a new branch
git switch --create backport-3446-to-release-2.11
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick --mainline 1 d4a41b41fd6c06f1db2673fbd499fb9cbd2a71d2
# Push it to GitHub
git push --set-upstream origin backport-3446-to-release-2.11
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release-2.11

Then, create a pull request where the base branch is release-2.11 and the compare/head branch is backport-3446-to-release-2.11.

KiterLuc added a commit that referenced this pull request Aug 21, 2022
* Fix reader serialization for old clients.

Old clients might submit a query using the legacy reader when the server
would select the refactored ones. This causes au issue because the read
state gets deserialized by the client and used to determine query
completion. The server needs to serialize a read state that the client
will understand, so for now, if the client decides to use the legacy
reader, the server needs to process that query with the legacy reader.

Long term, we will want to serialize a member of the query to determine
completion, and the read state will be serialized as a blob not
deserialized by the client. That way, the server can decide to use
whatever reader it wants.

---
TYPE: IMPROVEMENT
DESC: Fix reader serialization for old clients.

* Adding test coverage.
Shelnutt2 pushed a commit that referenced this pull request Aug 21, 2022
* Fix reader serialization for old clients.

Old clients might submit a query using the legacy reader when the server
would select the refactored ones. This causes au issue because the read
state gets deserialized by the client and used to determine query
completion. The server needs to serialize a read state that the client
will understand, so for now, if the client decides to use the legacy
reader, the server needs to process that query with the legacy reader.

Long term, we will want to serialize a member of the query to determine
completion, and the read state will be serialized as a blob not
deserialized by the client. That way, the server can decide to use
whatever reader it wants.

---
TYPE: IMPROVEMENT
DESC: Fix reader serialization for old clients.

* Adding test coverage.
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

2 participants