-
Notifications
You must be signed in to change notification settings - Fork 72
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
#1327 QueryBatcher now stops when query fails #1505
Conversation
490b47d
to
775feed
Compare
775feed
to
4c9f364
Compare
@@ -785,7 +779,15 @@ public void run() { | |||
isDone.set(true); | |||
shutdownIfAllForestsAreDone(); | |||
return; | |||
} | |||
} catch (Throwable t) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@llinggit I believe this catch
used to exist prior to this change for #1269, which resulted in this catch
being removed. The test I added captures the issue, which is that without this catch
, a failed query results in QueryBatcherImpl hanging indefinitely. I am not 100% sure of the contents of this block though, but it does result in the desired fix, which is that the job is stopped and control returns back to the user.
@@ -704,7 +703,6 @@ private class QueryTask implements Runnable { | |||
this.filtered = filtered; | |||
this.forestBatchNum = forestBatchNum; | |||
this.start = start; | |||
this.isQueryBatch = isQueryBatch; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed, isQueryBatch
is not used anywhere and this was assigning the variable to itself.
private QueryBatchImpl batch; | ||
private int totalProcessedCount = 0; | ||
private boolean isLastBatch; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed isLastBatch
and lastBatchNum
as neither were used anywhere.
// change date to now minus seven years | ||
date.roll(Calendar.YEAR, -7); | ||
String sevenYearsAgo = DatatypeConverter.printDateTime(date); | ||
// TODO This test failed when 2022 became 2023; increasing -7 to a higher number fixed it. The test could obviously |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including this here as this test started failing when 2022 became 2023.
4c5e756
to
4649905
Compare
Addresses DEVEXP-147 (internal bug). Also enabled a test that had been disabled due to this bug.
4649905
to
81d6a81
Compare
Addresses DEVEXP-147 (internal bug).