-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Improve refcounting in testClientCancellation
#110309
Improve refcounting in testClientCancellation
#110309
Conversation
With the changes in elastic#109519 we now do one more async step while serving the response, so we need to acquire another ref to track the new step. Relates elastic#109866 Relates elastic#110118 Relates elastic#110175 Relates elastic#110249
Pinging @elastic/es-distributed (Team:Distributed) |
…sIT-testClientCancellation
final var latch = new CountDownLatch(1); | ||
refs = LeakTracker.wrap(reachabilityChecker.register(AbstractRefCounted.of(latch::countDown))); | ||
refs = AbstractRefCounted.of(latch::countDown); |
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.
Are you sure it's not worth leaving the leak tracking in? it did make it much easier to troubleshoot.
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.
I'm not sure, no, although I'd generally rather keep debug stuff out of tests while they're not known to be flaky. If we keep having problems here then we can put it back easily enough.
Post-merge LGTM. |
With the changes in #109519 we now do one more async step while serving
the response, so we need to acquire another ref to track the new step.
Relates #109866
Relates #110118
Relates #110175
Relates #110249