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

Always half-close DoExchange snapshot requests #4632

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

niloc132
Copy link
Member

Partial mitigation for #4627

rcaudy
rcaudy previously approved these changes Oct 12, 2023
nbauernfeind
nbauernfeind previously approved these changes Oct 12, 2023
@niloc132
Copy link
Member Author

Something is wrong with this, do not merge.

@niloc132 niloc132 dismissed stale reviews from nbauernfeind and rcaudy via 3c033cd October 13, 2023 02:13
@niloc132
Copy link
Member Author

Okay, I misremembered how we implemented hierarchical table data - this actually is a subscription and not snapshot based, so it was incorrect to half close right away. As far as I can tell, the java client is the only official client that used the snapshot api in this way - JS api is the only other consumer, and it already half-closed right away (...and doesn't use h2 anyway for DoExchange).

@niloc132 niloc132 enabled auto-merge (squash) October 13, 2023 02:15
@niloc132 niloc132 merged commit 9275306 into deephaven:main Oct 13, 2023
10 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants