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