[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15741327#comment-15741327 ] Hadoop QA commented on ZOOKEEPER-2391: -- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12800423/ZOOKEEPER-2391-3.4-3.patch against trunk revision cd0e323831c8b4cde64976325bfc79bb53cdd9b7. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3546//console This message is automatically generated. > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Attachments: ZOOKEEPER-2391-2.patch, ZOOKEEPER-2391-3.4-2.patch, > ZOOKEEPER-2391-3.4-3.patch, ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391-3.patch, > ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15269284#comment-15269284 ] Patrick Hunt commented on ZOOKEEPER-2391: - bq. Is it possible to make Hadoop test the patch against 3.4 branch? Unfortunately not. QA bot only knows how to run against trunk, it's an unfortunate limitation. > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Fix For: 3.4.8, 3.5.0 > > Attachments: ZOOKEEPER-2391-2.patch, ZOOKEEPER-2391-3.4-2.patch, > ZOOKEEPER-2391-3.4-3.patch, ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391-3.patch, > ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15255644#comment-15255644 ] Kazuaki Banzai commented on ZOOKEEPER-2391: --- The test of ZOOKEEPER-2391-3.4-3.patch failed because Hadoop tested it against trunk. Is it possible to make Hadoop test the patch against 3.4 branch? > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Fix For: 3.4.8, 3.5.0 > > Attachments: ZOOKEEPER-2391-2.patch, ZOOKEEPER-2391-3.4-2.patch, > ZOOKEEPER-2391-3.4-3.patch, ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391-3.patch, > ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15255639#comment-15255639 ] Hadoop QA commented on ZOOKEEPER-2391: -- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12800423/ZOOKEEPER-2391-3.4-3.patch against trunk revision 1738012. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3144//console This message is automatically generated. > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Fix For: 3.4.8, 3.5.0 > > Attachments: ZOOKEEPER-2391-2.patch, ZOOKEEPER-2391-3.4-2.patch, > ZOOKEEPER-2391-3.4-3.patch, ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391-3.patch, > ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15255613#comment-15255613 ] Hadoop QA commented on ZOOKEEPER-2391: -- +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12800421/ZOOKEEPER-2391-3.patch against trunk revision 1738012. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3143//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3143//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3143//console This message is automatically generated. > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Fix For: 3.4.8, 3.5.0 > > Attachments: ZOOKEEPER-2391-2.patch, ZOOKEEPER-2391-3.4-2.patch, > ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391-3.patch, ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15253336#comment-15253336 ] Hadoop QA commented on ZOOKEEPER-2391: -- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12797219/ZOOKEEPER-2391-2.patch against trunk revision 1738012. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3142//console This message is automatically generated. > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Fix For: 3.4.8, 3.5.0 > > Attachments: ZOOKEEPER-2391-2.patch, ZOOKEEPER-2391-3.4-2.patch, > ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15227552#comment-15227552 ] Kazuaki Banzai commented on ZOOKEEPER-2391: --- Thank you for your code review. I have uploaded the modified version of the patches. > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Attachments: ZOOKEEPER-2391-2.patch, ZOOKEEPER-2391-3.4-2.patch, > ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[
https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15219201#comment-15219201
]
Edward Ribeiro commented on ZOOKEEPER-2391:
---
Hi [~kazuakibanzai], nice work. A couple of points regarding your patches:
1. It would be cool if you could follow the same code syntax rules of ZK
codebase. For example:
{code}
if(this.minSessionTimeout > maxSessionTimeout){
{code}
can be modified by inserting a space between the {{if}} and the {{(}} and
before the curly braces, as below:
{code}
if (this.minSessionTimeout > maxSessionTimeout) {
{code}
See the rest of the code in the same files you are changing for clues. ;)
2. Could you document the changes in the
{{src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml}} file? There are
sections devoted to maxSessionTimeout and minSessionTimeout there. When
compiled, those xml files generate the ZK pdf docs, so it would be cool if you
add a bit of info to reflect the code changes too (for example, what happens if
< -1, == 1, etc).
3. There's a small typo in the javadoc comment above {{public void
setMaxSessionTimeout(int max)}}:
{{max minSessionTimeout}} should be {{max maxSessionTimeout}} (copy-and-paste
artifact, I guess ;) )
4. Also, I'd suggest to beef up the javadoc comment. An example, but keep it in
my that this is only a rough *suggestion*, please adjust if you feel like doing
so:
{code}
/**
* @param max session timeout in milliseconds, if max value is -1 the
default value is set.
* @throws IllegalArgumentException if max timeout < -1 or min timeout >
max timeout
*/
{code}
> setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak
> way
> ---
>
> Key: ZOOKEEPER-2391
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391
> Project: ZooKeeper
> Issue Type: Bug
> Components: server
>Reporter: Kazuaki Banzai
>Assignee: Kazuaki Banzai
>Priority: Minor
> Attachments: ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391.patch
>
>
> setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak
> way.
> * -1 restores the default, but this is not documented.
> * values < -1 are permitted but make no sense.
> * min > max is permitted but makes not sense.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15218933#comment-15218933 ] Kazuaki Banzai commented on ZOOKEEPER-2391: --- I uploaded ZOOKEEPER-2391-3.4.patch for zookeeper 3.4 branch. ZOOKEEPER-2391.patch can be applied to 3.5 and trunk branch. I think the docs don't need to update in the patch, so I removed the documentation tag. > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Attachments: ZOOKEEPER-2391-3.4.patch, ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15203101#comment-15203101 ] Patrick Hunt commented on ZOOKEEPER-2391: - bq. Does "the docs" mean the comment of Javadoc or something else? I suspect Flavio was referring to the documentation proper. It's under src/docs/... Notice there is existing documentation for these parameters. bq. Should I test the code in 3.4 and 3.5? if you think it makes sense to commit to those branches then it's helpful if you apply the patch to them and check. Sometimes we need to submit separate patches because of conflicts... > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: documentation, server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Attachments: ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15200145#comment-15200145 ] Kazuaki Banzai commented on ZOOKEEPER-2391: --- Thank you for replying this report. This is my first bug report and I don't understand the process of contribution well. So please let me ask some questions. >>Do you want to update the docs as well in your patch? Does "the docs" mean the comment of Javadoc or something else? I added the documentation tag because I wrote Javadoc comment(I'm sorry if I misunderstand the use of the tag). >>Also, I think we need to fix this in branches 3.4, 3.5, and trunk, yes? Yes. I wrote this patch in trunk branch, but I didn't check the code in 3.4 and 3.5. Should I test the code in 3.4 and 3.5? > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: documentation, server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Attachments: ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (ZOOKEEPER-2391) setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak way
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2391?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15197298#comment-15197298 ] Flavio Junqueira commented on ZOOKEEPER-2391: - Thanks for reporting the issue and the patch, [~kazuakibanzai]. Do you want to update the docs as well in your patch? Also, I think we need to fix this in branches 3.4, 3.5, and trunk, yes? > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way > --- > > Key: ZOOKEEPER-2391 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2391 > Project: ZooKeeper > Issue Type: Bug > Components: documentation, server >Reporter: Kazuaki Banzai >Assignee: Kazuaki Banzai >Priority: Minor > Attachments: ZOOKEEPER-2391.patch > > > setMin/MaxSessionTimeout of ZookeeperServer are implemented in quite a weak > way. > * -1 restores the default, but this is not documented. > * values < -1 are permitted but make no sense. > * min > max is permitted but makes not sense. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
