Hi All, I created ZOOKEEPER-2393 and submitted patch. Please review Thanks & Regards Arshad
On Fri, Mar 18, 2016 at 12:28 AM, Chris Nauroth <[email protected]> wrote: > Hi Mohammad, > > If you want to file a JIRA with a patch for the partial revert, then > please go ahead. Feel free to notify me on the issue for code review. > Thanks! > > --Chris Nauroth > > > > > On 3/17/16, 12:23 AM, "Mohammad arshad" <[email protected]> > wrote: > > >>>1. Full revert of ZOOKEEPER-1371, targeted to 3.5.2. > >Can we do partial revert instead of full > > i) revert log4j and slf4j test scoped dependency to run time dependency, > >as it we earlier before this patch was merged > > ii) revert the changes done in build.xml > >For partial revert we can create another path on top of latest code, I > >can create the patch. > >This will avoid lot of duplicate effort > > > >>>2. Retarget ZOOKEEPER-1371 to 3.5.3 with the scope ... > >This is not required as if above is done > > > >>> 3. Retarget ZOOKEEPER-2342 to 3.6.0 ... > >+1 for this > > > > > >-----Original Message----- > >From: Flavio Junqueira [mailto:[email protected]] > >Sent: 17 March 2016 12:21 > >To: [email protected] > >Cc: Chris Nauroth > >Subject: Re: We need to prioritize getting logging fixed in trunk/3.5 > > > >Agreed, sounds like a good plan, +1. > > > >Thanks, Chris! > > > >-Flavio > > > >> On 17 Mar 2016, at 06:02, Patrick Hunt <[email protected]> wrote: > >> > >> That makes sense to me as well. +1. Thank you Chris! > >> > >> Patrick > >> > >> On Wed, Mar 16, 2016 at 9:42 AM, Raúl Gutiérrez Segalés > >><[email protected]> wrote: > >>> +1. This sounds like a good plan. Thanks Chris! > >>> On Mar 16, 2016 9:36 AM, "Chris Nauroth" <[email protected]> > >>>wrote: > >>> > >>>> We now have multiple binding +1's for a revert. To finalize the > >>>> plan, here is what I propose: > >>>> > >>>> 1. Full revert of ZOOKEEPER-1371, targeted to 3.5.2. > >>>> > >>>> 2. Retarget ZOOKEEPER-1371 to 3.5.3 with the scope limited to just > >>>> the SLF4J logging API changes. We'd omit the build changes that > >>>> dropped the SLF4J-Log4J 1.2 binding from the distro. This would be > >>>> a backwards-compatible change, and I believe it was the original > >>>> intent of ZOOKEEPER-1371. This is not critical to complete for > >>>> 3.5.3. I'm just pushing it ahead to the next version. > >>>> > >>>> 3. Retarget ZOOKEEPER-2342 to 3.6.0 for tracking Log4J 2 migration. > >>>> This will have to happen someday since Log4J 1 is end of life, but > >>>> it will likely be backwards-incompatible, and the change provides no > >>>> value add to justify it for the 3.5 line. > >>>> > >>>> I'll wait 24 hours before proceeding with a revert in case anyone > >>>> else wants to comment. > >>>> > >>>> --Chris Nauroth > >>>> > >>>> > >>>> > >>>> > >>>> On 3/16/16, 6:47 AM, "Camille Fournier" <[email protected]> wrote: > >>>> > >>>>> I'm a strong +1 to get this fixed even if it requires reverting the > >>>>> original patch. Broken logging is huge. Let's do whatever is > >>>>> expedient and sensible to fix it. > >>>>> > >>>>> On Tue, Mar 15, 2016 at 7:27 PM, Chris Nauroth > >>>>> <[email protected]> > >>>>> wrote: > >>>>> > >>>>>> Yes, that's basically my assessment too. Copy-pasting my earlier > >>>>>> comment from ZOOKEEPER-1371: > >>>>>> > >>>>>> "After this patch, ZooKeeper no longer produces any logging, > >>>>>> because there is no SLF4J binding jar available on the runtime > >>>>>> classpath." > >>>>>> > >>>>>> > >>>>>> There is no compatibility problem with switching to SLF4J > >>>>>> exclusively as our API of choice for logging instead of calling > >>>>>> the Log4J API. The incompatible part is that the distro isn't > >>>>>> shipping with any SLF4J binding included. Perhaps we can do a > >>>>>> partial revert of just that part of ZOOKEEPER-1371. > >>>>>> > >>>>>> --Chris Nauroth > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 3/15/16, 11:54 AM, "Patrick Hunt" <[email protected]> wrote: > >>>>>> > >>>>>>> Hm, I started looking at the original patch in more depth: > >>>>>>> > >>>>>> > >>>>>> > >>>> https://issues.apache.org/jira/secure/attachment/12773684/ZOOKEEPER- > >>>> 1371- > >>>>>> 0 > >>>>>>> 5.patch > >>>>>>> > >>>>>>> is the real root issue 2342 is trying to address the following > >>>>>>> line > >>>>>>> change: > >>>>>>> > >>>>>>> - <dependency org="org.slf4j" name="slf4j-log4j12" rev="1.7.5" > >>>>>>> transitive="false"/> > >>>>>>> + <dependency org="org.slf4j" name="slf4j-log4j12" rev="1.7.5" > >>>>>>> transitive="false" conf="test->default"/> > >>>>>>> > >>>>>>> Specifically that we changed from runtime to test only for this > >>>>>>> dependency? Perhaps we just need to revert that? I see some other > >>>>>>> magic happening in the build.xml file that I don't quite > >>>>>>> understand - adding a new target and NoLog4j... references. > >>>>>>> > >>>>>>> Raul perhaps you can give more insight since it seems like you > >>>>>>> worked on 1371 most recently? > >>>>>>> > >>>>>>> Patrick > >>>>>>> > >>>>>>> > >>>>>>> On Tue, Mar 15, 2016 at 11:41 AM, Chris Nauroth > >>>>>>> <[email protected]> wrote: > >>>>>>>> I agree. Even if we don't fully understand every minute > >>>>>>>> technical detail of Log4J 2 vs. Log4J 1, I think we've learned > >>>>>>>> enough from my work-in-progress patch to declare that a > >>>>>>>> migration is too risky for > >>>>>> the > >>>>>>>> 3.5 line. Reverting ZOOKEEPER-1371 (the earlier > >>>>>> backwards-incompatible > >>>>>>>> logging change) is the better choice for the interest of > >>>>>>>> proceeding > >>>>>> with > >>>>>>>> 3.5 releases. > >>>>>>>> > >>>>>>>> --Chris Nauroth > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On 3/15/16, 11:23 AM, "Patrick Hunt" <[email protected]> wrote: > >>>>>>>> > >>>>>>>>> I just commented on ZOOKEEPER-2342... not sure I fully > >>>>>>>>> understand all the issues to be honest. Given how much we're > >>>>>>>>> trying to do in 3.5 it seems like it would be prudent to wait > >>>>>>>>> on 1371 until 3.6... IMO. :-) > >>>>>>>>> > >>>>>>>>> Patrick > >>>>>>>>> > >>>>>>>>> On Tue, Mar 15, 2016 at 11:15 AM, Chris Nauroth > >>>>>>>>> <[email protected]> wrote: > >>>>>>>>>> At this point, I am +1 for a revert of the patch that > >>>>>>>>>> introduced > >>>>>> the > >>>>>>>>>> problem (ZOOKEEPER-1371). We need more time to come up with a > >>>>>>>>>> migration path to Log4J 2 that minimizes impact on operators. > >>>>>>>>>> That will take time, and I'd prefer that we don't hold up > >>>>>>>>>> 3.5.2-alpha for it. > >>>>>>>>>> > >>>>>>>>>> --Chris Nauroth > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On 3/15/16, 11:08 AM, "Patrick Hunt" <[email protected]> wrote: > >>>>>>>>>> > >>>>>>>>>>> Hi folks, can we prioritize getting logging fixed? It's > >>>>>>>>>>> causing > >>>>>> test > >>>>>>>>>>> failures, e.g.: > >>>>>>>>>>> > >>>>>> https://builds.apache.org/job/ZooKeeper-trunk/2850/artifact/trunk/ > >>>>>> buil > >>>>>>>>>>> d/ > >>>>>>>>>>> tm > >>>>>>>>>>> p/zk.log > >>>>>>>>>>> > >>>>>>>>>>> This is the jira: > >>>>>>>>>>> https://issues.apache.org/jira/browse/ZOOKEEPER-2342 > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> Perhaps we should revert the change that caused this in the > >>>>>>>>>>> first place. > >>>>>>>>>>> > >>>>>>>>>>> Patrick > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>> > >>>> > > > >
