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