Issue #1664 Cancel Scheduled SpeculativeReads #1665
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If configured every read request schedules a Future task to send speculative reads on speculativeReadTimeout. When the read is completed successfully, this task must be canceled otherwise it leads to memory consumption and under heavy load the tasks get accumulated which forces lengthy GC cycles. These lengthy GC cycles may cause ZK lease expiry and all other sorts of problems eventually resulting in application errors. This fix makes sure that the scheduled Futures are cancelled at the end of read task. Signed-off-by: Venkateswararao Jujjuri (JV) <vjujjuri@salesforce.com> (@ref Andrey@)
sijie
approved these changes
Sep 7, 2018
eolivelli
reviewed
Sep 7, 2018
| @@ -353,7 +373,7 @@ public void testSpeculativeReadScheduling() throws Exception { | |||
| } | |||
| Thread.sleep(1000); | |||
| } | |||
| assertTrue("Request should be done", req0.isComplete()); | |||
| assertTrue("Request should be done", req.isComplete()); | |||
|
@jvrao please fix spotbugs, then I will be happy to merge and cherry pick |
|
Java9 failure is on Twitter stats; Is that flaky or something I should look into? can we merge this? @sijie ? |
|
rerun java9 tests |
|
retest this please |
sijie
pushed a commit
that referenced
this pull request
Sep 10, 2018
Descriptions of the changes in this PR: If configured every read request schedules a Future task to send speculative reads on speculativeReadTimeout. When the read is completed successfully, this task must be canceled otherwise it leads to memory consumption and under heavy load the tasks get accumulated which forces lengthy GC cycles. These lengthy GC cycles may cause ZK lease expiry and all other sorts of problems eventually resulting in application errors. This fix makes sure that the scheduled Futures are cancelled at the end of read task. Signed-off-by: Venkateswararao Jujjuri (JV) <vjujjurisalesforce.com> (ref Andrey) Author: JV Jujjuri <vjujjuri@salesforce.com> Author: Sijie Guo <sijie@apache.org> Reviewers: Andrey Yegorov <None>, Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org> This closes #1665 from jvrao/ups_speculative_cancel, closes #1664 (cherry picked from commit cdd6594) Signed-off-by: Sijie Guo <sijie@apache.org>
sijie
pushed a commit
that referenced
this pull request
Sep 10, 2018
Descriptions of the changes in this PR: If configured every read request schedules a Future task to send speculative reads on speculativeReadTimeout. When the read is completed successfully, this task must be canceled otherwise it leads to memory consumption and under heavy load the tasks get accumulated which forces lengthy GC cycles. These lengthy GC cycles may cause ZK lease expiry and all other sorts of problems eventually resulting in application errors. This fix makes sure that the scheduled Futures are cancelled at the end of read task. Signed-off-by: Venkateswararao Jujjuri (JV) <vjujjurisalesforce.com> (ref Andrey) Author: JV Jujjuri <vjujjuri@salesforce.com> Author: Sijie Guo <sijie@apache.org> Reviewers: Andrey Yegorov <None>, Enrico Olivelli <eolivelli@gmail.com>, Sijie Guo <sijie@apache.org> This closes #1665 from jvrao/ups_speculative_cancel, closes #1664 (cherry picked from commit cdd6594) Signed-off-by: Sijie Guo <sijie@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If configured every read request schedules a Future task
to send speculative reads on speculativeReadTimeout.
When the read is completed successfully, this task must be
canceled otherwise it leads to memory consumption and under
heavy load the tasks get accumulated which forces lengthy
GC cycles. These lengthy GC cycles may cause ZK lease expiry
and all other sorts of problems eventually resulting in application
errors.
This fix makes sure that the scheduled Futures are cancelled
at the end of read task.
Signed-off-by: Venkateswararao Jujjuri (JV) vjujjuri@salesforce.com
(@ref Andrey@)
Descriptions of the changes in this PR:
Motivation
(Explain: why you're making that change, what is the problem you're trying to solve)
Changes
(Describe: what changes you have made)
Master Issue: #