Uh oh!
There was an error while loading. Please reload this page.
This repository was archived by the owner on Sep 26, 2023. It is now read-only.
Watchdog - Refactor - #482
Merged
garrettjonesgoogle merged 4 commits intoFeb 24, 2018
Merged
Conversation
- when the watchdog moves to ClientContext, it will be always on, so check interval must be nonnull - when the timeouts move to ApiCallContext, they be disabled via null not zero, so switch to those semantics - explicitly annotate everything else in ServerStreamingCallSetting for good measure
- Convert the watchdog into a Runnable that is scheduled externally - Make idleTimeout configureable per watch - Rename Watchdog.IdleConnectionException -> WatchdogTimeoutException and make it toplevel - pushdown the generics from the Watchdog class to the Watchdog#watch method
Codecov Report
@@ Coverage Diff @@## master #482 +/- ##
============================================
- Coverage 70.34% 70.33% -0.01% - Complexity 773 775 +2
============================================
Files 162 163 +1 Lines 3672 3668 -4 Branches 275 273 -2 ============================================
- Hits 2583 2580 -3
Misses 973 973 + Partials 116 115 -1
Continue to review full report at Codecov.
|
garrettjonesgoogle
left a comment
Contributor
There was a problem hiding this comment.
This approach seems reasonable.
garrettjonesgoogle
commented
Feb 19, 2018
Contributor
@vam-google PTAL also |
| Duration rpcTimeout = outerRetryingFuture.getAttemptSettings().getRpcTimeout(); | ||
| if (rpcTimeout.isZero()) { | ||
| rpcTimeout = null; |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
| public class Watchdog<ResponseT> { | ||
| public class Watchdog implements Runnable { | ||
| // Dummy value to convert the ConcurrentHashMap into a Set | ||
| private static Object VALUE_MARKER = new Object(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
igorbernstein2
commented
Feb 22, 2018
ContributorAuthor
Updated, PTAL |
igorbernstein2
commented
Feb 23, 2018
ContributorAuthor
@garrettjonesgoogle PTAL and if it looks ok, please merge and I will rebase #480 |
This file contains hidden or 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
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
This is in preparation for #480.
when the watchdog moves to ClientContext, it will be on by default, so check interval should be nonnullwhen the timeouts move to ApiCallContext, they can be disabled via null not zero, so switch to those semanticsUpdates:
This is ready for review. @garrettjonesgoogle PTAL