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 >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>>> >>> >>>
