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: erroneous QueryCompiler retry when nothing unaccounted for #6451

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

nbauernfeind
Copy link
Member

Fixes #6216.

I discovered how to reproduce this by accident (my script was mid-write). Using a debugger I found that the retry list was empty, meaning that all of the non-failing items were successfully written to disk.

@nbauernfeind nbauernfeind added bug Something isn't working query engine core Core development tasks NoDocumentationNeeded ReleaseNotesNeeded Release notes are needed point_release_candidate labels Nov 29, 2024
@nbauernfeind nbauernfeind self-assigned this Nov 29, 2024
@cpwright
Copy link
Contributor

Is there a reasonable way to write a unit test for this?

@nbauernfeind
Copy link
Member Author

Is there a reasonable way to write a unit test for this?

It seems like it might be jdk specific. TestQueryCompiler#testMultiCompileWithFailure simulates the scenario where this is reproduced. When I originally implemented parallel compilation, and it seems is true for CI as well, when there was one failure then all compilation units failed. That required a second loop through the compiler to get the passing ones to write out their class files.

When I reproduced on Friday, it tuned out that all of the non-failing units were properly written to disk causing this retry list to be empty; so I'm not certain how to reproduce in the CI environment.

@nbauernfeind nbauernfeind changed the title Fix erroneous QueryCompiler retry when nothing unaccounted for fix: erroneous QueryCompiler retry when nothing unaccounted for Dec 2, 2024
@nbauernfeind
Copy link
Member Author

Actually, it does not seem to be jvm dependent. It is dependent on the ordering of the requests. Unit test incoming.

@nbauernfeind
Copy link
Member Author

nbauernfeind commented Dec 2, 2024

Apparently, the type of error matters; some errors appear to not be able to continue whereas other errors may be overlooked. I've updated the unit tests where the new one fails before the bugfix.

@nbauernfeind nbauernfeind merged commit 3827ba0 into deephaven:main Dec 2, 2024
17 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working core Core development tasks NoDocumentationNeeded point_release_candidate query engine ReleaseNotesNeeded Release notes are needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QueryCompiler IllegalStateException in Embedded Server - "no source files"
2 participants