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: Allows clients to expect that query performance results have been logged before they receive a response #6334

Merged
merged 5 commits into from
Nov 5, 2024

Conversation

rcaudy
Copy link
Member

@rcaudy rcaudy commented Nov 4, 2024

Defer response messaging to onSuccess for all ExportBuilders that use QueryPerformanceRecorder. This allows clients to expect that query performance results have been logged before they receive a response.

Change summary:

  • Adjust all SessionState.ExportBuilder uses that set a QueryPerformanceRecorder to use onSuccess for response delivery if possible
  • Exclusions: DoExchange subscriptions and async input table operations
  • Note: batch already used onSuccess, but completion now happens after performance results are reported
  • Some cleanup to GrpcUtil usage

…eRecorder to use onSuccess for response delivery if possible. Exclusions: DoExchange subscriptions and async input table operations. Exceptions: batch already used onSuccess.
@rcaudy rcaudy added this to the 0.37.0 milestone Nov 4, 2024
@rcaudy rcaudy self-assigned this Nov 4, 2024
@rcaudy rcaudy changed the title Defer response messaging to onSuccess for all ExportBuilders that use QueryPerformanceRecorder fix: Defer response messaging to onSuccess for all ExportBuilders that use QueryPerformanceRecorder Nov 4, 2024
nbauernfeind
nbauernfeind previously approved these changes Nov 4, 2024
@rcaudy rcaudy changed the title fix: Defer response messaging to onSuccess for all ExportBuilders that use QueryPerformanceRecorder fix: Alows clients to expect that query performance results have been logged before they receive a response Nov 5, 2024
@rcaudy rcaudy enabled auto-merge (squash) November 5, 2024 19:10
@nbauernfeind
Copy link
Member

LGTM, waiting for a ping from @niloc132 before approving in case he wants a final look before the merge.

@rcaudy rcaudy merged commit 7657ff5 into deephaven:main Nov 5, 2024
19 checks passed
@rcaudy rcaudy deleted the rwc-onsuccess branch November 5, 2024 23:47
@github-actions github-actions bot locked and limited conversation to collaborators Nov 5, 2024
@nbauernfeind nbauernfeind changed the title fix: Alows clients to expect that query performance results have been logged before they receive a response fix: Allows clients to expect that query performance results have been logged before they receive a response Nov 6, 2024
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.

2 participants