[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[ https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13925391#comment-13925391 ] Sijie Guo commented on BOOKKEEPER-602: -- committed as r1575792 in branch 4.2. we should have request timeouts rather than channel timeout in PerChannelBookieClient - Key: BOOKKEEPER-602 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602 Project: Bookkeeper Issue Type: Bug Components: bookkeeper-client Affects Versions: 4.2.0, 4.2.1 Reporter: Sijie Guo Assignee: Aniruddha Fix For: 4.3.0, 4.2.3 Attachments: 0001-BOOKKEEPER-602-we-should-have-request-timeouts-rathe.patch, BOOKKEEPER-602.diff, BOOKKEEPER-602.diff, BOOKKEEPER-602_branch42.diff currently we only have readTimeout in netty channel, it timeouts only when there is no activities in that channel, but it can't track timeouts of individual requests. if a channel continues having read entry activities, it might shadow a slow add entry response, which is bad impacting add latency. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[
https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13807440#comment-13807440
]
Hadoop QA commented on BOOKKEEPER-602:
--
Testing JIRA BOOKKEEPER-602
Patch
[0001-BOOKKEEPER-602-we-should-have-request-timeouts-rathe.patch|https://issues.apache.org/jira/secure/attachment/12610664/0001-BOOKKEEPER-602-we-should-have-request-timeouts-rathe.patch]
downloaded at Mon Oct 28 22:26:39 UTC 2013
{color:green}+1 PATCH_APPLIES{color}
{color:green}+1 CLEAN{color}
{color:green}+1 RAW_PATCH_ANALYSIS{color}
.{color:green}+1{color} the patch does not introduce any @author tags
.{color:green}+1{color} the patch does not introduce any tabs
.{color:green}+1{color} the patch does not introduce any trailing spaces
.{color:green}+1{color} the patch does not introduce any line longer than
120
.{color:green}+1{color} the patch does adds/modifies 6 testcase(s)
{color:green}+1 RAT{color}
.{color:green}+1{color} the patch does not seem to introduce new RAT
warnings
{color:green}+1 JAVADOC{color}
.{color:green}+1{color} the patch does not seem to introduce new Javadoc
warnings
{color:green}+1 COMPILE{color}
.{color:green}+1{color} HEAD compiles
.{color:green}+1{color} patch compiles
.{color:green}+1{color} the patch does not seem to introduce new javac
warnings
{color:green}+1 FINDBUGS{color}
.{color:green}+1{color} the patch does not seem to introduce new Findbugs
warnings
{color:green}+1 TESTS{color}
.Tests run: 882
{color:green}+1 DISTRO{color}
.{color:green}+1{color} distro tarball builds with the patch
{color:green}*+1 Overall result, good!, no -1s*{color}
The full output of the test-patch run is available at
. https://builds.apache.org/job/bookkeeper-trunk-precommit-build/514/
we should have request timeouts rather than channel timeout in
PerChannelBookieClient
-
Key: BOOKKEEPER-602
URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602
Project: Bookkeeper
Issue Type: Bug
Affects Versions: 4.2.0, 4.2.1
Reporter: Sijie Guo
Assignee: Aniruddha
Fix For: 4.3.0
Attachments:
0001-BOOKKEEPER-602-we-should-have-request-timeouts-rathe.patch,
BOOKKEEPER-602.diff, BOOKKEEPER-602.diff
currently we only have readTimeout in netty channel, it timeouts only when
there is no activities in that channel, but it can't track timeouts of
individual requests. if a channel continues having read entry activities, it
might shadow a slow add entry response, which is bad impacting add latency.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[ https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13807659#comment-13807659 ] Sijie Guo commented on BOOKKEEPER-602: -- +1 commiting we should have request timeouts rather than channel timeout in PerChannelBookieClient - Key: BOOKKEEPER-602 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602 Project: Bookkeeper Issue Type: Bug Affects Versions: 4.2.0, 4.2.1 Reporter: Sijie Guo Assignee: Aniruddha Fix For: 4.3.0 Attachments: 0001-BOOKKEEPER-602-we-should-have-request-timeouts-rathe.patch, BOOKKEEPER-602.diff, BOOKKEEPER-602.diff currently we only have readTimeout in netty channel, it timeouts only when there is no activities in that channel, but it can't track timeouts of individual requests. if a channel continues having read entry activities, it might shadow a slow add entry response, which is bad impacting add latency. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[ https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13807672#comment-13807672 ] Hudson commented on BOOKKEEPER-602: --- SUCCESS: Integrated in bookkeeper-trunk #418 (See [https://builds.apache.org/job/bookkeeper-trunk/418/]) BOOKKEEPER-602: we should have request timeouts rather than channel timeout in PerChannelBookieClient (Aniruddha, ivank via sijie) (sijie: rev 1536584) * /zookeeper/bookkeeper/trunk/CHANGES.txt * /zookeeper/bookkeeper/trunk/bookkeeper-benchmark/src/main/java/org/apache/bookkeeper/benchmark/BenchReadThroughputLatency.java * /zookeeper/bookkeeper/trunk/bookkeeper-benchmark/src/main/java/org/apache/bookkeeper/benchmark/BenchThroughputLatency.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCloseTest.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/SlowBookieTest.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestReadTimeout.java * /zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestSpeculativeRead.java * /zookeeper/bookkeeper/trunk/hedwig-server/src/test/java/org/apache/hedwig/server/persistence/TestBookKeeperPersistenceManager.java * /zookeeper/bookkeeper/trunk/hedwig-server/src/test/java/org/apache/hedwig/server/persistence/TestDeadlock.java we should have request timeouts rather than channel timeout in PerChannelBookieClient - Key: BOOKKEEPER-602 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602 Project: Bookkeeper Issue Type: Bug Affects Versions: 4.2.0, 4.2.1 Reporter: Sijie Guo Assignee: Aniruddha Fix For: 4.3.0 Attachments: 0001-BOOKKEEPER-602-we-should-have-request-timeouts-rathe.patch, BOOKKEEPER-602.diff, BOOKKEEPER-602.diff currently we only have readTimeout in netty channel, it timeouts only when there is no activities in that channel, but it can't track timeouts of individual requests. if a channel continues having read entry activities, it might shadow a slow add entry response, which is bad impacting add latency. -- This message was sent by Atlassian JIRA (v6.1#6144)
[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[ https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13719292#comment-13719292 ] Sijie Guo commented on BOOKKEEPER-602: -- The reason why we put getAddEntryTimeout to 1 is to put the best parameter for most of cases. so the user doesn't need to tune it too much. based on this consideration, we'd prefer putting the tuned value as default value and having TestBKConfiguration to handle low-throughput case (this TestBKConfiguration also exists in the journal improvements we made in BOOKKEEPER-657). if you feel strongly about it, I could change. let me know. 1 second is based on performance evaluation, 1) we don't want a slow add request to cause too much pending requests accumulated in client (which cause bad gc behavior) 2) for latency consideration. the failure test is also related to the configuration setting. I forgot to bring the changes for hedwig when generating the patch. will add soon. we should have request timeouts rather than channel timeout in PerChannelBookieClient - Key: BOOKKEEPER-602 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602 Project: Bookkeeper Issue Type: Bug Affects Versions: 4.2.0, 4.2.1 Reporter: Sijie Guo Assignee: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-602.diff currently we only have readTimeout in netty channel, it timeouts only when there is no activities in that channel, but it can't track timeouts of individual requests. if a channel continues having read entry activities, it might shadow a slow add entry response, which is bad impacting add latency. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[ https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13713850#comment-13713850 ] Ivan Kelly commented on BOOKKEEPER-602: --- The patch looks mostly good. Setting the getAddEntryTimeout() to 1 is too low though, and a behavioural break. It should default to the value of getReadTimeout(), which should also be deprecated. The fact that you had to create a TestBKConfiguration shows that it breaks current behaviour. Also, the test failure needs to be fixed. we should have request timeouts rather than channel timeout in PerChannelBookieClient - Key: BOOKKEEPER-602 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602 Project: Bookkeeper Issue Type: Bug Affects Versions: 4.2.0, 4.2.1 Reporter: Sijie Guo Fix For: 4.3.0 Attachments: BOOKKEEPER-602.diff currently we only have readTimeout in netty channel, it timeouts only when there is no activities in that channel, but it can't track timeouts of individual requests. if a channel continues having read entry activities, it might shadow a slow add entry response, which is bad impacting add latency. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[
https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13701489#comment-13701489
]
Hadoop QA commented on BOOKKEEPER-602:
--
Testing JIRA BOOKKEEPER-602
Patch
[BOOKKEEPER-602.diff|https://issues.apache.org/jira/secure/attachment/12591098/BOOKKEEPER-602.diff]
downloaded at Sun Jul 7 02:37:36 UTC 2013
{color:green}+1 PATCH_APPLIES{color}
{color:green}+1 CLEAN{color}
{color:green}+1 RAW_PATCH_ANALYSIS{color}
.{color:green}+1{color} the patch does not introduce any @author tags
.{color:green}+1{color} the patch does not introduce any tabs
.{color:green}+1{color} the patch does not introduce any trailing spaces
.{color:green}+1{color} the patch does not introduce any line longer than
120
.{color:green}+1{color} the patch does adds/modifies 3 testcase(s)
{color:green}+1 RAT{color}
.{color:green}+1{color} the patch does not seem to introduce new RAT
warnings
{color:green}+1 JAVADOC{color}
.{color:green}+1{color} the patch does not seem to introduce new Javadoc
warnings
{color:green}+1 COMPILE{color}
.{color:green}+1{color} HEAD compiles
.{color:green}+1{color} patch compiles
.{color:green}+1{color} the patch does not seem to introduce new javac
warnings
{color:green}+1 FINDBUGS{color}
.{color:green}+1{color} the patch does not seem to introduce new Findbugs
warnings
{color:red}-1 TESTS{color}
.Tests run: 853
.Tests failed: 1
.Tests errors: 0
.The patch failed the following testcases:
. testDeadlock(org.apache.hedwig.server.persistence.TestDeadlock)
{color:green}+1 DISTRO{color}
.{color:green}+1{color} distro tarball builds with the patch
{color:red}*-1 Overall result, please check the reported -1(s)*{color}
The full output of the test-patch run is available at
. https://builds.apache.org/job/bookkeeper-trunk-precommit-build/416/
we should have request timeouts rather than channel timeout in
PerChannelBookieClient
-
Key: BOOKKEEPER-602
URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602
Project: Bookkeeper
Issue Type: Bug
Affects Versions: 4.2.0, 4.2.1
Reporter: Sijie Guo
Fix For: 4.3.0
Attachments: BOOKKEEPER-602.diff
currently we only have readTimeout in netty channel, it timeouts only when
there is no activities in that channel, but it can't track timeouts of
individual requests. if a channel continues having read entry activities, it
might shadow a slow add entry response, which is bad impacting add latency.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[ https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13623967#comment-13623967 ] Ivan Kelly commented on BOOKKEEPER-602: --- I think the original implementation did have a timer per request. The problem was this meant a thread per request. That said, this could easily be implemented with a scheduled executor instead of a timer. we should have request timeouts rather than channel timeout in PerChannelBookieClient - Key: BOOKKEEPER-602 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602 Project: Bookkeeper Issue Type: Bug Affects Versions: 4.2.0, 4.2.1 Reporter: Sijie Guo Fix For: 4.3.0 currently we only have readTimeout in netty channel, it timeouts only when there is no activities in that channel, but it can't track timeouts of individual requests. if a channel continues having read entry activities, it might shadow a slow add entry response, which is bad impacting add latency. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (BOOKKEEPER-602) we should have request timeouts rather than channel timeout in PerChannelBookieClient
[ https://issues.apache.org/jira/browse/BOOKKEEPER-602?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13623969#comment-13623969 ] Sijie Guo commented on BOOKKEEPER-602: -- yes. would attach the patch later. we should have request timeouts rather than channel timeout in PerChannelBookieClient - Key: BOOKKEEPER-602 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-602 Project: Bookkeeper Issue Type: Bug Affects Versions: 4.2.0, 4.2.1 Reporter: Sijie Guo Fix For: 4.3.0 currently we only have readTimeout in netty channel, it timeouts only when there is no activities in that channel, but it can't track timeouts of individual requests. if a channel continues having read entry activities, it might shadow a slow add entry response, which is bad impacting add latency. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
