Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-28 Thread Jordan Zimmerman
I pulled in the build file changes and that fixed the tests - good news.

So, https://github.com/apache/zookeeper/pull/377 
 is ready.

-Jordan

> On Sep 27, 2017, at 11:07 PM, Jordan Zimmerman  
> wrote:
> 
> This is on Jenkins. 
> 
> https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1051/testReport/
>  
> 
> 
>> On Sep 27, 2017, at 11:06 PM, Patrick Hunt  wrote:
>> 
>> Check your classpath (typ the build/libs and build/test/libs directories) - 
>> how many log4j jar files do you have? Are there conflicting versions? (same 
>> jar diff versions I mean).
>> 
>> Patrick
>> 
>> On Wed, Sep 27, 2017 at 8:57 PM, Jordan Zimmerman 
>> > wrote:
>> Now I'm getting a different error:
>> 
>> 2017-09-28 03:47:24,878 [myid:2] - ERROR [Thread-1:AppenderDynamicMBean@209] 
>> - Could not add DynamicLayoutMBean for 
>> [CONSOLE,layout=org.apache.log4j.PatternLayout].
>> javax.management.InstanceAlreadyExistsException: 
>> log4j:appender=CONSOLE,layout=org.apache.log4j.PatternLayout
>> 
>> 
>>> On Sep 27, 2017, at 1:17 PM, Jordan Zimmerman >> > wrote:
>>> 
>>> I didn't change anything. I branched from master. What should I do any 
>>> ideas?
>>> 
 On Sep 27, 2017, at 1:15 PM, Patrick Hunt > wrote:
 
 Has the log4j configuration changed at all? iirc the console appender 
 needs to be setup for those tests to function.
 
 Patrick
 
 On Sat, Sep 23, 2017 at 8:01 AM, Jordan Zimmerman 
 > wrote:
 There are 4 tests throwing NPEs in Jenkins due to:
 
 Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
 .getLayout();
 
 Is this a known issue? Any workaround?
 
 -Jordan
 
> On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman  > wrote:
> 
> In LeaderSessionTracker.java there is this bit of code:
> 
> if (!localSessionsEnabled
> || (getServerIdFromSessionId(sessionId) == serverId)) {
> throw new SessionExpiredException();
> }
> 
> "serverId" is a long. This can only work if Server IDs are 255 or less. I 
> realize this is in the docs. But is it enforced? See: 
> https://issues.apache.org/jira/browse/ZOOKEEPER-2503 
> 
> 
> 
> 
>> On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés 
>> > wrote:
>> 
>> On 20 September 2017 at 12:54, Camille Fournier > > wrote:
>> Ok let's take this back to either public mailing list or jira. I'd write 
>> up
>> thoughts on jira and ask there+ml to look. I'll try to look tonight
>> 
>> Thanks Camille!
>> 
>> Also, I merged this originally so I will work with Jordan on getting 
>> this fixed. Let me know
>> when you have a write up of your proposed solution and I'll take a look. 
>> Thanks!
>> 
>> 
>> -rgs
>>  
>> 
>> 
>> On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" > >
>> wrote:
>> 
>> > I'd like to fix it as my company and probably many others are now 
>> > using it
>> > in production. The question is how to fix it safely and correctly. Is 
>> > email
>> > the best way to discuss this? Jira? Something else?
>> >
>> > I must say that there appears to be a trivial fix but I need the ZK
>> > committers to think about this. In 
>> > SessionTrackerImpl#initializeNextSession()
>> > only some of the server ID bits are used. We could easily just mask 
>> > the 2
>> > high bits as well. But, what are the implications of this? Where is 
>> > this
>> > serverId byte used? What must be double checked?
>> >
>> > -Jordan
>> >
>> > On Sep 20, 2017, at 2:46 PM, Camille Fournier > > > wrote:
>> >
>> > Would you rather roll back the feature or put in a fix?
>> >
>> > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman" 
>> > >
>> > wrote:
>> >
>> >> Hey Folks,
>> >>
>> >> This is very serious. Please - let's discuss immediately. I'm not 
>> >> certain
>> >> how to fix this.
>> >>
>> >> -JZ
>> >>
>> >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman 
>> >> 

Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-28 Thread Abraham Fine
I submitted a JIRA (with patches) to resolve the OWASP related logging
issue here: https://issues.apache.org/jira/browse/ZOOKEEPER-2906
Thanks,
Abe


On Wed, Sep 27, 2017, at 21:37, Patrick Hunt wrote:
> I looked at the console output for the job you linked and there
> are tons of> 
>  [exec] junit.run-concurrent:
>  [exec]  [echo] Running 8 concurrent JUnit processes.
>  [exec] [junit] SLF4J: Class path contains multiple SLF4J
>  bindings.>  [exec] [junit] SLF4J: Found binding in 
> [jar:file:/home/jenkins/jenkins-slave/workspace/PreCommit-
>  ZOOKEEPER-github-pr-build/build/lib/dependency-check-ant-
>  2.1.0.jar!/org/slf4j/impl/StaticLoggerBinder.class]>  [exec] 
> [junit] SLF4J: Found binding in 
> [jar:file:/home/jenkins/jenkins-slave/workspace/PreCommit-
>  ZOOKEEPER-github-pr-build/build/lib/slf4j-log4j12-
>  1.7.5.jar!/org/slf4j/impl/StaticLoggerBinder.class]>  [exec] 
> [junit] SLF4J: See
>  http://www.slf4j.org/codes.html#multiple_bindings for an
>  explanation.>  [exec] [junit] SLF4J: Actual binding is of type
>  [org.owasp.dependencycheck.ant.logging.AntLoggerFactory]> 
> Looks like this is related to the owasp changes that were incorporated
> recently. Try stubbing that out of your patch (perhaps git revert it
> in your branch) and see if it resolves this from the console output
> and whether it helps at all with the issue you've been seeing in the
> test itself.> 
> Patrick
> 
> On Wed, Sep 27, 2017 at 9:07 PM, Jordan Zimmerman
>  wrote:>> This is on Jenkins.
>> 
>> https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1051/testReport/>>
>>  > On Sep 27, 2017, at 11:06 PM, Patrick Hunt 
>>  > wrote:
>>  >
>>  > Check your classpath (typ the build/libs and build/test/libs
>>  > directories) - how many log4j jar files do you have? Are there
>>  > conflicting versions? (same jar diff versions I mean).
>>  >
>>  > Patrick
>>  >
>> > On Wed, Sep 27, 2017 at 8:57 PM, Jordan Zimmerman
>> > >
>> > wrote:
>>  > Now I'm getting a different error:
>>  >
>>  > 2017-09-28 03:47:24,878 [myid:2] - ERROR [Thread-
>>  > 1:AppenderDynamicMBean@209] - Could not add DynamicLayoutMBean for
>>  > [CONSOLE,layout=org.apache.log4j.PatternLayout].
>>  > javax.management.InstanceAlreadyExistsException:
>>  > log4j:appender=CONSOLE,layout=org.apache.log4j.PatternLayout
>>  >
>>  >
>> >> On Sep 27, 2017, at 1:17 PM, Jordan Zimmerman
>> >> >
>> >> wrote:
>>  >>
>>  >> I didn't change anything. I branched from master. What should I
>>  >> do any ideas?
>>  >>
>> >>> On Sep 27, 2017, at 1:15 PM, Patrick Hunt > >>> > wrote:
>>  >>>
>>  >>> Has the log4j configuration changed at all? iirc the console
>>  >>> appender needs to be setup for those tests to function.
>>  >>>
>>  >>> Patrick
>>  >>>
>> >>> On Sat, Sep 23, 2017 at 8:01 AM, Jordan Zimmerman
>> >>> >
>> >>> wrote:
>>  >>> There are 4 tests throwing NPEs in Jenkins due to:
>>  >>>
>>  >>> Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
>>  >>> .getLayout();
>>  >>>
>>  >>> Is this a known issue? Any workaround?
>>  >>>
>>  >>> -Jordan
>>  >>>
>>  On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman
>>  >
>>  wrote:
>>  
>>   In LeaderSessionTracker.java there is this bit of code:
>>  
>>   if (!localSessionsEnabled
>>   || (getServerIdFromSessionId(sessionId) ==
>>   || serverId)) {
>>   throw new SessionExpiredException(); }
>>  
>>  "serverId" is a long. This can only work if Server IDs are 255
>>  or less. I realize this is in the docs. But is it enforced? See:
>>  https://issues.apache.org/jira/browse/ZOOKEEPER-2503
>>  >>  
>>  
>>  
>>  > On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés
>>  > > wrote:>> >
>>  > On 20 September 2017 at 12:54, Camille Fournier
>>  > > wrote: Ok
>>  > let's take this back to either public mailing list or jira.
>>  > I'd write up thoughts on jira and ask there+ml to look. I'll
>>  > try to look tonight
>>  >
>>  > Thanks Camille!
>>  >
>>  > Also, I merged this originally so I will work with Jordan on
>>  > getting this fixed. Let me know when you have a write up of
>>  > your proposed solution and I'll take a look. Thanks!
>>  >
>>  >
>>  > -rgs
>>  >
>>  >
>>  >
>> > On Sep 20, 2017 3:52 PM, "Jordan Zimmerman"
>> > 

Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-27 Thread Patrick Hunt
I looked at the console output for the job you linked and there are tons of

 [exec] junit.run-concurrent:
 [exec]  [echo] Running 8 concurrent JUnit processes.
 [exec] [junit] SLF4J: Class path contains multiple SLF4J bindings.
 [exec] [junit] SLF4J: Found binding in
[jar:file:/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build/lib/dependency-check-ant-2.1.0.jar!/org/slf4j/impl/StaticLoggerBinder.class]
 [exec] [junit] SLF4J: Found binding in
[jar:file:/home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build/build/lib/slf4j-log4j12-1.7.5.jar!/org/slf4j/impl/StaticLoggerBinder.class]
 [exec] [junit] SLF4J: See
http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
 [exec] [junit] SLF4J: Actual binding is of type
[org.owasp.dependencycheck.ant.logging.AntLoggerFactory]

Looks like this is related to the owasp changes that were incorporated
recently. Try stubbing that out of your patch (perhaps git revert it in
your branch) and see if it resolves this from the console output and
whether it helps at all with the issue you've been seeing in the test
itself.

Patrick

On Wed, Sep 27, 2017 at 9:07 PM, Jordan Zimmerman <
jor...@jordanzimmerman.com> wrote:

> This is on Jenkins.
>
> https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-
> build/1051/testReport/
>
> > On Sep 27, 2017, at 11:06 PM, Patrick Hunt  wrote:
> >
> > Check your classpath (typ the build/libs and build/test/libs
> directories) - how many log4j jar files do you have? Are there conflicting
> versions? (same jar diff versions I mean).
> >
> > Patrick
> >
> > On Wed, Sep 27, 2017 at 8:57 PM, Jordan Zimmerman <
> jor...@jordanzimmerman.com > wrote:
> > Now I'm getting a different error:
> >
> > 2017-09-28 03:47:24,878 [myid:2] - ERROR [Thread-1:
> AppenderDynamicMBean@209] - Could not add DynamicLayoutMBean for
> [CONSOLE,layout=org.apache.log4j.PatternLayout].
> > javax.management.InstanceAlreadyExistsException:
> log4j:appender=CONSOLE,layout=org.apache.log4j.PatternLayout
> >
> >
> >> On Sep 27, 2017, at 1:17 PM, Jordan Zimmerman <
> jor...@jordanzimmerman.com > wrote:
> >>
> >> I didn't change anything. I branched from master. What should I do any
> ideas?
> >>
> >>> On Sep 27, 2017, at 1:15 PM, Patrick Hunt > wrote:
> >>>
> >>> Has the log4j configuration changed at all? iirc the console appender
> needs to be setup for those tests to function.
> >>>
> >>> Patrick
> >>>
> >>> On Sat, Sep 23, 2017 at 8:01 AM, Jordan Zimmerman <
> jor...@jordanzimmerman.com > wrote:
> >>> There are 4 tests throwing NPEs in Jenkins due to:
> >>>
> >>> Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
> >>> .getLayout();
> >>>
> >>> Is this a known issue? Any workaround?
> >>>
> >>> -Jordan
> >>>
>  On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman <
> jor...@jordanzimmerman.com > wrote:
> 
>  In LeaderSessionTracker.java there is this bit of code:
> 
>  if (!localSessionsEnabled
>  || (getServerIdFromSessionId(sessionId) ==
> serverId)) {
>  throw new SessionExpiredException();
>  }
> 
>  "serverId" is a long. This can only work if Server IDs are 255 or
> less. I realize this is in the docs. But is it enforced? See:
> https://issues.apache.org/jira/browse/ZOOKEEPER-2503 <
> https://issues.apache.org/jira/browse/ZOOKEEPER-2503>
> 
> 
> 
> > On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés <
> r...@itevenworks.net > wrote:
> >
> > On 20 September 2017 at 12:54, Camille Fournier  > wrote:
> > Ok let's take this back to either public mailing list or jira. I'd
> write up
> > thoughts on jira and ask there+ml to look. I'll try to look tonight
> >
> > Thanks Camille!
> >
> > Also, I merged this originally so I will work with Jordan on getting
> this fixed. Let me know
> > when you have a write up of your proposed solution and I'll take a
> look. Thanks!
> >
> >
> > -rgs
> >
> >
> >
> > On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" <
> jor...@jordanzimmerman.com >
> > wrote:
> >
> > > I'd like to fix it as my company and probably many others are now
> using it
> > > in production. The question is how to fix it safely and correctly.
> Is email
> > > the best way to discuss this? Jira? Something else?
> > >
> > > I must say that there appears to be a trivial fix but I need the ZK
> > > committers to think about this. In SessionTrackerImpl#
> initializeNextSession()
> > > only some of the server ID bits are used. We could easily just
> mask 

Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-27 Thread Jordan Zimmerman
This is on Jenkins. 

https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/1051/testReport/

> On Sep 27, 2017, at 11:06 PM, Patrick Hunt  wrote:
> 
> Check your classpath (typ the build/libs and build/test/libs directories) - 
> how many log4j jar files do you have? Are there conflicting versions? (same 
> jar diff versions I mean).
> 
> Patrick
> 
> On Wed, Sep 27, 2017 at 8:57 PM, Jordan Zimmerman  > wrote:
> Now I'm getting a different error:
> 
> 2017-09-28 03:47:24,878 [myid:2] - ERROR [Thread-1:AppenderDynamicMBean@209] 
> - Could not add DynamicLayoutMBean for 
> [CONSOLE,layout=org.apache.log4j.PatternLayout].
> javax.management.InstanceAlreadyExistsException: 
> log4j:appender=CONSOLE,layout=org.apache.log4j.PatternLayout
> 
> 
>> On Sep 27, 2017, at 1:17 PM, Jordan Zimmerman > > wrote:
>> 
>> I didn't change anything. I branched from master. What should I do any ideas?
>> 
>>> On Sep 27, 2017, at 1:15 PM, Patrick Hunt >> > wrote:
>>> 
>>> Has the log4j configuration changed at all? iirc the console appender needs 
>>> to be setup for those tests to function.
>>> 
>>> Patrick
>>> 
>>> On Sat, Sep 23, 2017 at 8:01 AM, Jordan Zimmerman 
>>> > wrote:
>>> There are 4 tests throwing NPEs in Jenkins due to:
>>> 
>>> Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
>>> .getLayout();
>>> 
>>> Is this a known issue? Any workaround?
>>> 
>>> -Jordan
>>> 
 On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman > wrote:
 
 In LeaderSessionTracker.java there is this bit of code:
 
 if (!localSessionsEnabled
 || (getServerIdFromSessionId(sessionId) == serverId)) {
 throw new SessionExpiredException();
 }
 
 "serverId" is a long. This can only work if Server IDs are 255 or less. I 
 realize this is in the docs. But is it enforced? See: 
 https://issues.apache.org/jira/browse/ZOOKEEPER-2503 
 
 
 
 
> On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés  > wrote:
> 
> On 20 September 2017 at 12:54, Camille Fournier  > wrote:
> Ok let's take this back to either public mailing list or jira. I'd write 
> up
> thoughts on jira and ask there+ml to look. I'll try to look tonight
> 
> Thanks Camille!
> 
> Also, I merged this originally so I will work with Jordan on getting this 
> fixed. Let me know
> when you have a write up of your proposed solution and I'll take a look. 
> Thanks!
> 
> 
> -rgs
>  
> 
> 
> On Sep 20, 2017 3:52 PM, "Jordan Zimmerman"  >
> wrote:
> 
> > I'd like to fix it as my company and probably many others are now using 
> > it
> > in production. The question is how to fix it safely and correctly. Is 
> > email
> > the best way to discuss this? Jira? Something else?
> >
> > I must say that there appears to be a trivial fix but I need the ZK
> > committers to think about this. In 
> > SessionTrackerImpl#initializeNextSession()
> > only some of the server ID bits are used. We could easily just mask the 
> > 2
> > high bits as well. But, what are the implications of this? Where is this
> > serverId byte used? What must be double checked?
> >
> > -Jordan
> >
> > On Sep 20, 2017, at 2:46 PM, Camille Fournier  > > wrote:
> >
> > Would you rather roll back the feature or put in a fix?
> >
> > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman"  > >
> > wrote:
> >
> >> Hey Folks,
> >>
> >> This is very serious. Please - let's discuss immediately. I'm not 
> >> certain
> >> how to fix this.
> >>
> >> -JZ
> >>
> >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman 
> >> >
> >> wrote:
> >>
> >> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901 
> >> 
> >>
> >> It appears that the high order byte of a session ID is reserved for the
> >> ServerID. I don't know how I could have missed this or how this got by 
> >> code
> >> review, but Container Nodes and TTL nodes are using the 2 high bits to
> >> denote container/TTL. I'll work on a fix ASAP. But, 

Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-27 Thread Patrick Hunt
Check your classpath (typ the build/libs and build/test/libs directories) -
how many log4j jar files do you have? Are there conflicting versions? (same
jar diff versions I mean).

Patrick

On Wed, Sep 27, 2017 at 8:57 PM, Jordan Zimmerman <
jor...@jordanzimmerman.com> wrote:

> Now I'm getting a different error:
>
> 2017-09-28 03:47:24,878 [myid:2] - ERROR [Thread-1:AppenderDynamicMBean@209] 
> - Could not add DynamicLayoutMBean for 
> [CONSOLE,layout=org.apache.log4j.PatternLayout].
> javax.management.InstanceAlreadyExistsException: 
> log4j:appender=CONSOLE,layout=org.apache.log4j.PatternLayout
>
>
>
> On Sep 27, 2017, at 1:17 PM, Jordan Zimmerman 
> wrote:
>
> I didn't change anything. I branched from master. What should I do any
> ideas?
>
> On Sep 27, 2017, at 1:15 PM, Patrick Hunt  wrote:
>
> Has the log4j configuration changed at all? iirc the console appender
> needs to be setup for those tests to function.
>
> Patrick
>
> On Sat, Sep 23, 2017 at 8:01 AM, Jordan Zimmerman <
> jor...@jordanzimmerman.com> wrote:
>
>> There are 4 tests throwing NPEs in Jenkins due to:
>>
>> Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
>> .getLayout();
>>
>>
>> Is this a known issue? Any workaround?
>>
>> -Jordan
>>
>> On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman 
>> wrote:
>>
>> In LeaderSessionTracker.java there is this bit of code:
>>
>> if (!localSessionsEnabled
>> || (getServerIdFromSessionId(sessionId) == serverId)) {
>> throw new SessionExpiredException();
>> }
>>
>> "serverId" is a long. This can only work if Server IDs are 255 or less. I
>> realize this is in the docs. But is it enforced? See:
>> https://issues.apache.org/jira/browse/ZOOKEEPER-2503
>>
>>
>>
>> On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés 
>> wrote:
>>
>> On 20 September 2017 at 12:54, Camille Fournier 
>> wrote:
>>
>>> Ok let's take this back to either public mailing list or jira. I'd write
>>> up
>>> thoughts on jira and ask there+ml to look. I'll try to look tonight
>>>
>>
>> Thanks Camille!
>>
>> Also, I merged this originally so I will work with Jordan on getting this
>> fixed. Let me know
>> when you have a write up of your proposed solution and I'll take a look.
>> Thanks!
>>
>>
>> -rgs
>>
>>
>>
>>> On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" 
>>> wrote:
>>>
>>> > I'd like to fix it as my company and probably many others are now
>>> using it
>>> > in production. The question is how to fix it safely and correctly. Is
>>> email
>>> > the best way to discuss this? Jira? Something else?
>>> >
>>> > I must say that there appears to be a trivial fix but I need the ZK
>>> > committers to think about this. In SessionTrackerImpl#initializeN
>>> extSession()
>>> > only some of the server ID bits are used. We could easily just mask
>>> the 2
>>> > high bits as well. But, what are the implications of this? Where is
>>> this
>>> > serverId byte used? What must be double checked?
>>> >
>>> > -Jordan
>>> >
>>> > On Sep 20, 2017, at 2:46 PM, Camille Fournier 
>>> wrote:
>>> >
>>> > Would you rather roll back the feature or put in a fix?
>>> >
>>> > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman" <
>>> jor...@jordanzimmerman.com>
>>> > wrote:
>>> >
>>> >> Hey Folks,
>>> >>
>>> >> This is very serious. Please - let's discuss immediately. I'm not
>>> certain
>>> >> how to fix this.
>>> >>
>>> >> -JZ
>>> >>
>>> >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman <
>>> jor...@jordanzimmerman.com>
>>> >> wrote:
>>> >>
>>> >> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901
>>> >>
>>> >> It appears that the high order byte of a session ID is reserved for
>>> the
>>> >> ServerID. I don't know how I could have missed this or how this got
>>> by code
>>> >> review, but Container Nodes and TTL nodes are using the 2 high bits to
>>> >> denote container/TTL. I'll work on a fix ASAP. But, can someone
>>> validate
>>> >> this?
>>> >>
>>> >> -Jordan
>>> >>
>>> >>
>>> >>
>>> >
>>>
>>
>>
>>
>>
>
>
>


Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-27 Thread Jordan Zimmerman
Now I'm getting a different error:

2017-09-28 03:47:24,878 [myid:2] - ERROR [Thread-1:AppenderDynamicMBean@209] - 
Could not add DynamicLayoutMBean for 
[CONSOLE,layout=org.apache.log4j.PatternLayout].
javax.management.InstanceAlreadyExistsException: 
log4j:appender=CONSOLE,layout=org.apache.log4j.PatternLayout


> On Sep 27, 2017, at 1:17 PM, Jordan Zimmerman  
> wrote:
> 
> I didn't change anything. I branched from master. What should I do any ideas?
> 
>> On Sep 27, 2017, at 1:15 PM, Patrick Hunt > > wrote:
>> 
>> Has the log4j configuration changed at all? iirc the console appender needs 
>> to be setup for those tests to function.
>> 
>> Patrick
>> 
>> On Sat, Sep 23, 2017 at 8:01 AM, Jordan Zimmerman 
>> > wrote:
>> There are 4 tests throwing NPEs in Jenkins due to:
>> 
>> Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
>> .getLayout();
>> 
>> Is this a known issue? Any workaround?
>> 
>> -Jordan
>> 
>>> On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman >> > wrote:
>>> 
>>> In LeaderSessionTracker.java there is this bit of code:
>>> 
>>> if (!localSessionsEnabled
>>> || (getServerIdFromSessionId(sessionId) == serverId)) {
>>> throw new SessionExpiredException();
>>> }
>>> 
>>> "serverId" is a long. This can only work if Server IDs are 255 or less. I 
>>> realize this is in the docs. But is it enforced? See: 
>>> https://issues.apache.org/jira/browse/ZOOKEEPER-2503 
>>> 
>>> 
>>> 
>>> 
 On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés > wrote:
 
 On 20 September 2017 at 12:54, Camille Fournier > wrote:
 Ok let's take this back to either public mailing list or jira. I'd write up
 thoughts on jira and ask there+ml to look. I'll try to look tonight
 
 Thanks Camille!
 
 Also, I merged this originally so I will work with Jordan on getting this 
 fixed. Let me know
 when you have a write up of your proposed solution and I'll take a look. 
 Thanks!
 
 
 -rgs
  
 
 
 On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" >
 wrote:
 
 > I'd like to fix it as my company and probably many others are now using 
 > it
 > in production. The question is how to fix it safely and correctly. Is 
 > email
 > the best way to discuss this? Jira? Something else?
 >
 > I must say that there appears to be a trivial fix but I need the ZK
 > committers to think about this. In 
 > SessionTrackerImpl#initializeNextSession()
 > only some of the server ID bits are used. We could easily just mask the 2
 > high bits as well. But, what are the implications of this? Where is this
 > serverId byte used? What must be double checked?
 >
 > -Jordan
 >
 > On Sep 20, 2017, at 2:46 PM, Camille Fournier  > wrote:
 >
 > Would you rather roll back the feature or put in a fix?
 >
 > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman"  >
 > wrote:
 >
 >> Hey Folks,
 >>
 >> This is very serious. Please - let's discuss immediately. I'm not 
 >> certain
 >> how to fix this.
 >>
 >> -JZ
 >>
 >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman 
 >> >
 >> wrote:
 >>
 >> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901 
 >> 
 >>
 >> It appears that the high order byte of a session ID is reserved for the
 >> ServerID. I don't know how I could have missed this or how this got by 
 >> code
 >> review, but Container Nodes and TTL nodes are using the 2 high bits to
 >> denote container/TTL. I'll work on a fix ASAP. But, can someone validate
 >> this?
 >>
 >> -Jordan
 >>
 >>
 >>
 >
 
>>> 
>> 
>> 
> 



Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-27 Thread Jordan Zimmerman
I didn't change anything. I branched from master. What should I do any ideas?

> On Sep 27, 2017, at 1:15 PM, Patrick Hunt  wrote:
> 
> Has the log4j configuration changed at all? iirc the console appender needs 
> to be setup for those tests to function.
> 
> Patrick
> 
> On Sat, Sep 23, 2017 at 8:01 AM, Jordan Zimmerman  > wrote:
> There are 4 tests throwing NPEs in Jenkins due to:
> 
> Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
> .getLayout();
> 
> Is this a known issue? Any workaround?
> 
> -Jordan
> 
>> On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman > > wrote:
>> 
>> In LeaderSessionTracker.java there is this bit of code:
>> 
>> if (!localSessionsEnabled
>> || (getServerIdFromSessionId(sessionId) == serverId)) {
>> throw new SessionExpiredException();
>> }
>> 
>> "serverId" is a long. This can only work if Server IDs are 255 or less. I 
>> realize this is in the docs. But is it enforced? See: 
>> https://issues.apache.org/jira/browse/ZOOKEEPER-2503 
>> 
>> 
>> 
>> 
>>> On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés >> > wrote:
>>> 
>>> On 20 September 2017 at 12:54, Camille Fournier >> > wrote:
>>> Ok let's take this back to either public mailing list or jira. I'd write up
>>> thoughts on jira and ask there+ml to look. I'll try to look tonight
>>> 
>>> Thanks Camille!
>>> 
>>> Also, I merged this originally so I will work with Jordan on getting this 
>>> fixed. Let me know
>>> when you have a write up of your proposed solution and I'll take a look. 
>>> Thanks!
>>> 
>>> 
>>> -rgs
>>>  
>>> 
>>> 
>>> On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" >> >
>>> wrote:
>>> 
>>> > I'd like to fix it as my company and probably many others are now using it
>>> > in production. The question is how to fix it safely and correctly. Is 
>>> > email
>>> > the best way to discuss this? Jira? Something else?
>>> >
>>> > I must say that there appears to be a trivial fix but I need the ZK
>>> > committers to think about this. In 
>>> > SessionTrackerImpl#initializeNextSession()
>>> > only some of the server ID bits are used. We could easily just mask the 2
>>> > high bits as well. But, what are the implications of this? Where is this
>>> > serverId byte used? What must be double checked?
>>> >
>>> > -Jordan
>>> >
>>> > On Sep 20, 2017, at 2:46 PM, Camille Fournier >> > > wrote:
>>> >
>>> > Would you rather roll back the feature or put in a fix?
>>> >
>>> > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman" >> > >
>>> > wrote:
>>> >
>>> >> Hey Folks,
>>> >>
>>> >> This is very serious. Please - let's discuss immediately. I'm not certain
>>> >> how to fix this.
>>> >>
>>> >> -JZ
>>> >>
>>> >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman 
>>> >> >
>>> >> wrote:
>>> >>
>>> >> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901 
>>> >> 
>>> >>
>>> >> It appears that the high order byte of a session ID is reserved for the
>>> >> ServerID. I don't know how I could have missed this or how this got by 
>>> >> code
>>> >> review, but Container Nodes and TTL nodes are using the 2 high bits to
>>> >> denote container/TTL. I'll work on a fix ASAP. But, can someone validate
>>> >> this?
>>> >>
>>> >> -Jordan
>>> >>
>>> >>
>>> >>
>>> >
>>> 
>> 
> 
> 



Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-27 Thread Patrick Hunt
Has the log4j configuration changed at all? iirc the console appender needs
to be setup for those tests to function.

Patrick

On Sat, Sep 23, 2017 at 8:01 AM, Jordan Zimmerman <
jor...@jordanzimmerman.com> wrote:

> There are 4 tests throwing NPEs in Jenkins due to:
>
> Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
> .getLayout();
>
>
> Is this a known issue? Any workaround?
>
> -Jordan
>
> On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman 
> wrote:
>
> In LeaderSessionTracker.java there is this bit of code:
>
> if (!localSessionsEnabled
> || (getServerIdFromSessionId(sessionId) == serverId)) {
> throw new SessionExpiredException();
> }
>
> "serverId" is a long. This can only work if Server IDs are 255 or less. I
> realize this is in the docs. But is it enforced? See:
> https://issues.apache.org/jira/browse/ZOOKEEPER-2503
>
>
>
> On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés 
> wrote:
>
> On 20 September 2017 at 12:54, Camille Fournier 
> wrote:
>
>> Ok let's take this back to either public mailing list or jira. I'd write
>> up
>> thoughts on jira and ask there+ml to look. I'll try to look tonight
>>
>
> Thanks Camille!
>
> Also, I merged this originally so I will work with Jordan on getting this
> fixed. Let me know
> when you have a write up of your proposed solution and I'll take a look.
> Thanks!
>
>
> -rgs
>
>
>
>> On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" 
>> wrote:
>>
>> > I'd like to fix it as my company and probably many others are now using
>> it
>> > in production. The question is how to fix it safely and correctly. Is
>> email
>> > the best way to discuss this? Jira? Something else?
>> >
>> > I must say that there appears to be a trivial fix but I need the ZK
>> > committers to think about this. In SessionTrackerImpl#initializeN
>> extSession()
>> > only some of the server ID bits are used. We could easily just mask the
>> 2
>> > high bits as well. But, what are the implications of this? Where is this
>> > serverId byte used? What must be double checked?
>> >
>> > -Jordan
>> >
>> > On Sep 20, 2017, at 2:46 PM, Camille Fournier 
>> wrote:
>> >
>> > Would you rather roll back the feature or put in a fix?
>> >
>> > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman" > >
>> > wrote:
>> >
>> >> Hey Folks,
>> >>
>> >> This is very serious. Please - let's discuss immediately. I'm not
>> certain
>> >> how to fix this.
>> >>
>> >> -JZ
>> >>
>> >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman <
>> jor...@jordanzimmerman.com>
>> >> wrote:
>> >>
>> >> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901
>> >>
>> >> It appears that the high order byte of a session ID is reserved for the
>> >> ServerID. I don't know how I could have missed this or how this got by
>> code
>> >> review, but Container Nodes and TTL nodes are using the 2 high bits to
>> >> denote container/TTL. I'll work on a fix ASAP. But, can someone
>> validate
>> >> this?
>> >>
>> >> -Jordan
>> >>
>> >>
>> >>
>> >
>>
>
>
>
>


Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-23 Thread Jordan Zimmerman
There are 4 tests throwing NPEs in Jenkins due to:

Layout layout = Logger.getRootLogger().getAppender("CONSOLE")
.getLayout();

Is this a known issue? Any workaround?

-Jordan

> On Sep 21, 2017, at 9:17 AM, Jordan Zimmerman  
> wrote:
> 
> In LeaderSessionTracker.java there is this bit of code:
> 
> if (!localSessionsEnabled
> || (getServerIdFromSessionId(sessionId) == serverId)) {
> throw new SessionExpiredException();
> }
> 
> "serverId" is a long. This can only work if Server IDs are 255 or less. I 
> realize this is in the docs. But is it enforced? See: 
> https://issues.apache.org/jira/browse/ZOOKEEPER-2503 
> 
> 
> 
> 
>> On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés > > wrote:
>> 
>> On 20 September 2017 at 12:54, Camille Fournier > > wrote:
>> Ok let's take this back to either public mailing list or jira. I'd write up
>> thoughts on jira and ask there+ml to look. I'll try to look tonight
>> 
>> Thanks Camille!
>> 
>> Also, I merged this originally so I will work with Jordan on getting this 
>> fixed. Let me know
>> when you have a write up of your proposed solution and I'll take a look. 
>> Thanks!
>> 
>> 
>> -rgs
>>  
>> 
>> 
>> On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" > >
>> wrote:
>> 
>> > I'd like to fix it as my company and probably many others are now using it
>> > in production. The question is how to fix it safely and correctly. Is email
>> > the best way to discuss this? Jira? Something else?
>> >
>> > I must say that there appears to be a trivial fix but I need the ZK
>> > committers to think about this. In 
>> > SessionTrackerImpl#initializeNextSession()
>> > only some of the server ID bits are used. We could easily just mask the 2
>> > high bits as well. But, what are the implications of this? Where is this
>> > serverId byte used? What must be double checked?
>> >
>> > -Jordan
>> >
>> > On Sep 20, 2017, at 2:46 PM, Camille Fournier > > > wrote:
>> >
>> > Would you rather roll back the feature or put in a fix?
>> >
>> > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman" > > >
>> > wrote:
>> >
>> >> Hey Folks,
>> >>
>> >> This is very serious. Please - let's discuss immediately. I'm not certain
>> >> how to fix this.
>> >>
>> >> -JZ
>> >>
>> >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman > >> >
>> >> wrote:
>> >>
>> >> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901 
>> >> 
>> >>
>> >> It appears that the high order byte of a session ID is reserved for the
>> >> ServerID. I don't know how I could have missed this or how this got by 
>> >> code
>> >> review, but Container Nodes and TTL nodes are using the 2 high bits to
>> >> denote container/TTL. I'll work on a fix ASAP. But, can someone validate
>> >> this?
>> >>
>> >> -Jordan
>> >>
>> >>
>> >>
>> >
>> 
> 



Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-21 Thread Jordan Zimmerman
In LeaderSessionTracker.java there is this bit of code:

if (!localSessionsEnabled
|| (getServerIdFromSessionId(sessionId) == serverId)) {
throw new SessionExpiredException();
}

"serverId" is a long. This can only work if Server IDs are 255 or less. I 
realize this is in the docs. But is it enforced? See: 
https://issues.apache.org/jira/browse/ZOOKEEPER-2503 




> On Sep 20, 2017, at 3:10 PM, Raúl Gutiérrez Segalés  
> wrote:
> 
> On 20 September 2017 at 12:54, Camille Fournier  > wrote:
> Ok let's take this back to either public mailing list or jira. I'd write up
> thoughts on jira and ask there+ml to look. I'll try to look tonight
> 
> Thanks Camille!
> 
> Also, I merged this originally so I will work with Jordan on getting this 
> fixed. Let me know
> when you have a write up of your proposed solution and I'll take a look. 
> Thanks!
> 
> 
> -rgs
>  
> 
> 
> On Sep 20, 2017 3:52 PM, "Jordan Zimmerman"  >
> wrote:
> 
> > I'd like to fix it as my company and probably many others are now using it
> > in production. The question is how to fix it safely and correctly. Is email
> > the best way to discuss this? Jira? Something else?
> >
> > I must say that there appears to be a trivial fix but I need the ZK
> > committers to think about this. In 
> > SessionTrackerImpl#initializeNextSession()
> > only some of the server ID bits are used. We could easily just mask the 2
> > high bits as well. But, what are the implications of this? Where is this
> > serverId byte used? What must be double checked?
> >
> > -Jordan
> >
> > On Sep 20, 2017, at 2:46 PM, Camille Fournier  > > wrote:
> >
> > Would you rather roll back the feature or put in a fix?
> >
> > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman"  > >
> > wrote:
> >
> >> Hey Folks,
> >>
> >> This is very serious. Please - let's discuss immediately. I'm not certain
> >> how to fix this.
> >>
> >> -JZ
> >>
> >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman  >> >
> >> wrote:
> >>
> >> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901 
> >> 
> >>
> >> It appears that the high order byte of a session ID is reserved for the
> >> ServerID. I don't know how I could have missed this or how this got by code
> >> review, but Container Nodes and TTL nodes are using the 2 high bits to
> >> denote container/TTL. I'll work on a fix ASAP. But, can someone validate
> >> this?
> >>
> >> -Jordan
> >>
> >>
> >>
> >
> 



Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-20 Thread Raúl Gutiérrez Segalés
On 20 September 2017 at 12:54, Camille Fournier  wrote:

> Ok let's take this back to either public mailing list or jira. I'd write up
> thoughts on jira and ask there+ml to look. I'll try to look tonight
>

Thanks Camille!

Also, I merged this originally so I will work with Jordan on getting this
fixed. Let me know
when you have a write up of your proposed solution and I'll take a look.
Thanks!


-rgs



> On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" 
> wrote:
>
> > I'd like to fix it as my company and probably many others are now using
> it
> > in production. The question is how to fix it safely and correctly. Is
> email
> > the best way to discuss this? Jira? Something else?
> >
> > I must say that there appears to be a trivial fix but I need the ZK
> > committers to think about this. In SessionTrackerImpl#
> initializeNextSession()
> > only some of the server ID bits are used. We could easily just mask the 2
> > high bits as well. But, what are the implications of this? Where is this
> > serverId byte used? What must be double checked?
> >
> > -Jordan
> >
> > On Sep 20, 2017, at 2:46 PM, Camille Fournier 
> wrote:
> >
> > Would you rather roll back the feature or put in a fix?
> >
> > On Sep 20, 2017 3:44 PM, "Jordan Zimmerman" 
> > wrote:
> >
> >> Hey Folks,
> >>
> >> This is very serious. Please - let's discuss immediately. I'm not
> certain
> >> how to fix this.
> >>
> >> -JZ
> >>
> >> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman <
> jor...@jordanzimmerman.com>
> >> wrote:
> >>
> >> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901
> >>
> >> It appears that the high order byte of a session ID is reserved for the
> >> ServerID. I don't know how I could have missed this or how this got by
> code
> >> review, but Container Nodes and TTL nodes are using the 2 high bits to
> >> denote container/TTL. I'll work on a fix ASAP. But, can someone validate
> >> this?
> >>
> >> -Jordan
> >>
> >>
> >>
> >
>


Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-20 Thread Camille Fournier
Ok let's take this back to either public mailing list or jira. I'd write up
thoughts on jira and ask there+ml to look. I'll try to look tonight

On Sep 20, 2017 3:52 PM, "Jordan Zimmerman" 
wrote:

> I'd like to fix it as my company and probably many others are now using it
> in production. The question is how to fix it safely and correctly. Is email
> the best way to discuss this? Jira? Something else?
>
> I must say that there appears to be a trivial fix but I need the ZK
> committers to think about this. In SessionTrackerImpl#initializeNextSession()
> only some of the server ID bits are used. We could easily just mask the 2
> high bits as well. But, what are the implications of this? Where is this
> serverId byte used? What must be double checked?
>
> -Jordan
>
> On Sep 20, 2017, at 2:46 PM, Camille Fournier  wrote:
>
> Would you rather roll back the feature or put in a fix?
>
> On Sep 20, 2017 3:44 PM, "Jordan Zimmerman" 
> wrote:
>
>> Hey Folks,
>>
>> This is very serious. Please - let's discuss immediately. I'm not certain
>> how to fix this.
>>
>> -JZ
>>
>> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman 
>> wrote:
>>
>> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901
>>
>> It appears that the high order byte of a session ID is reserved for the
>> ServerID. I don't know how I could have missed this or how this got by code
>> review, but Container Nodes and TTL nodes are using the 2 high bits to
>> denote container/TTL. I'll work on a fix ASAP. But, can someone validate
>> this?
>>
>> -Jordan
>>
>>
>>
>


Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-20 Thread Jordan Zimmerman
I'd like to fix it as my company and probably many others are now using it in 
production. The question is how to fix it safely and correctly. Is email the 
best way to discuss this? Jira? Something else?

I must say that there appears to be a trivial fix but I need the ZK committers 
to think about this. In SessionTrackerImpl#initializeNextSession() only some of 
the server ID bits are used. We could easily just mask the 2 high bits as well. 
But, what are the implications of this? Where is this serverId byte used? What 
must be double checked?

-Jordan

> On Sep 20, 2017, at 2:46 PM, Camille Fournier  wrote:
> 
> Would you rather roll back the feature or put in a fix?
> 
> On Sep 20, 2017 3:44 PM, "Jordan Zimmerman"  > wrote:
> Hey Folks,
> 
> This is very serious. Please - let's discuss immediately. I'm not certain how 
> to fix this.
> 
> -JZ
> 
>> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman > > wrote:
>> 
>> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901 
>> 
>> 
>> It appears that the high order byte of a session ID is reserved for the 
>> ServerID. I don't know how I could have missed this or how this got by code 
>> review, but Container Nodes and TTL nodes are using the 2 high bits to 
>> denote container/TTL. I'll work on a fix ASAP. But, can someone validate 
>> this?
>> 
>> -Jordan
> 



Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-20 Thread Camille Fournier
Would you rather roll back the feature or put in a fix?

On Sep 20, 2017 3:44 PM, "Jordan Zimmerman" 
wrote:

> Hey Folks,
>
> This is very serious. Please - let's discuss immediately. I'm not certain
> how to fix this.
>
> -JZ
>
> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman 
> wrote:
>
> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901
>
> It appears that the high order byte of a session ID is reserved for the
> ServerID. I don't know how I could have missed this or how this got by code
> review, but Container Nodes and TTL nodes are using the 2 high bits to
> denote container/TTL. I'll work on a fix ASAP. But, can someone validate
> this?
>
> -Jordan
>
>
>


Re: Major issue with Container Nodes/TTL nodes!!!

2017-09-20 Thread Jordan Zimmerman
Hey Folks,

This is very serious. Please - let's discuss immediately. I'm not certain how 
to fix this.

-JZ

> On Sep 20, 2017, at 2:17 PM, Jordan Zimmerman  
> wrote:
> 
> See: https://issues.apache.org/jira/browse/ZOOKEEPER-2901 
> 
> 
> It appears that the high order byte of a session ID is reserved for the 
> ServerID. I don't know how I could have missed this or how this got by code 
> review, but Container Nodes and TTL nodes are using the 2 high bits to denote 
> container/TTL. I'll work on a fix ASAP. But, can someone validate this?
> 
> -Jordan