-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
KAFKA-6741: Disable Selector's idle connection timeout in testNetworkThreadTimeRecorded() test #4824
Conversation
…kThreadTimeRecorded() test
@rajinisivaram trace logs shows that connection is getting closed due to idle time out. increased the idle connection timeout to 10secs for this test alone. Pls take a look.
|
@@ -635,7 +635,7 @@ public void testApplicationBufferResize() throws Exception { | |||
@Test | |||
public void testNetworkThreadTimeRecorded() throws Exception { | |||
selector.close(); | |||
this.selector = new Selector(NetworkReceive.UNLIMITED, 5000, new Metrics(), Time.SYSTEM, | |||
this.selector = new Selector(NetworkReceive.UNLIMITED, 10000, new Metrics(), Time.SYSTEM, |
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.
Would it make sense to disable idle timeout with NO_IDLE_TIMEOUT_MS
for this test? Do we want it to be at play?
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.
Agree, We can set timeout to NO_IDLE_TIMEOUT_MS for this test.
@omkreddy Is the test failing consistently without this fix and passing consistently with this fix? The test is waiting for a receive and if it fails to receive, then it would hit idle timeout since there is no other traffic, but I am not sure idle timeout is the cause of the failure. There could be some other timing issue that is causing the receive to fail. |
…hreadTimeRecorded() test
@rajinisivaram Yes, test is failing most of the times without this patch and passing consistently with the fix. ran 100 times without any error. so i thought, may be system slowness/load is causing the test to fail. |
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.
@omkreddy Thanks for the PR, LGTM.
@omkreddy Thanks for the PR, merging to trunk. |
…ThreadTimeRecorded() test (apache#4824) Reviewers: Jason Gustafson <jason@confluent.io>, Rajini Sivaram <rajinisivaram@googlemail.com>
No description provided.