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