sfl4j as our front-end logger just went in (HBASE-10092) thanks to Balazs
Meszaros code-fu and our Appy review (with some timely input from our
comrades over in slf4j2).
There'll be some teething issues to be addressed in the next few days but
going forward, from here on out, as we go, we need to convert as we go our
log messages from being 'guarded' -- i.e. if (LOG.isDebugEnabled...) -- to
instead being parameterized log messages. e.g. the latter rather than the
former in the below:
logger.debug("The new entry is "+entry+".");
logger.debug("The new entry is {}.", entry);
See [1] for background on perf benefits.
Note, FATAL log level is not present in slf4j.
Next up, changing our logging backend (probably log4j2 after we figure how
to bring our operators over from a .properties to an .xml config file --
hbase3?).
Yours,
S
1. https://www.slf4j.org/faq.html#logging_performance
On Tue, Dec 19, 2017 at 9:19 AM, Stack <st...@duboce.net> wrote:
> On Tue, Dec 19, 2017 at 7:29 AM, Balazs Meszaros <
> balazs.mesza...@cloudera.com> wrote:
>
>> Thanks for reviewing Appy!
>>
>> 1. I tried to verify it, log level changes take place through the web ui.
>> 2. I put back fatals.
>> 3. The property files are still compatible, because I have not updated
>> log4j to log4j2 yet. But they won't be compatible after the update.
>>
>
> We are not going to log4j2. You thinking otherwise Balazs?
>
>
>
>> 4. I also updated those projects.
>>
>> Unfortunately there are some issues which need to be solved before
>> updating
>> to log4j2:
>> 1. There are still some references in our java files to log4j (e.g.
>> LogManager references). We use it to set log levels from the code:
>> - on the web ui,
>>
>
> Looking at hadoop3, they seem to keep log4j around in the loglevel servlet
> at least just for changing log levels. We can do same?
>
>
>> - in the unit tests (I removed them, because there were no asserts on
>> the
>> log messages),
>> - some command line tools also configured log levels from the code.
>> 2. hbase-http also uses log4j for request logging.
>>
>> There are downsides to having two systems? hbase-http at least is in its
> module.
>
>
>> If we can't rid off from these dependencies, then our codebase won't be
>> completely independent from the logging implementation.
>>
>>
> Sounds like we can't do a pure cut-over, more like a 95%. Thats good
> enough i'd say.
>
> Thanks for the work Balazs,
> St.Ack
>
>
>
>
>> Best regards,
>> Balazs
>>
>> On Tue, Dec 19, 2017 at 2:37 AM, Apekshit Sharma <a...@cloudera.com>
>> wrote:
>>
>> > Thanks for the ping here Stack. Posted review on the jira.
>> > Summary is, we need at least:
>> > 1) basic verification
>> > 2) fatal markers
>> > 3) clear picture on properties file: Is old one compatible? if not, we
>> need
>> > new ones and document what's breaking. New ones and documentation can be
>> > done in followup, but we need clear picture now.
>> > 4) Followup jira for hbase-backup/http or a reason why they can't be
>> done.
>> >
>> > Reverting this patch would be hell, wouldn't want to do it because we
>> > missed basic checks.
>> >
>> > -- Appy
>> >
>> >
>> > On Mon, Dec 18, 2017 at 3:53 PM, Apekshit Sharma <a...@cloudera.com>
>> > wrote:
>> >
>> > > Oh, just 60 pages of review :D
>> > >
>> > > -- Appy
>> > >
>> > > On Mon, Dec 18, 2017 at 3:48 PM, Stack <st...@duboce.net> wrote:
>> > >
>> > >> On Wed, Dec 13, 2017 at 10:09 AM, Stack <st...@duboce.net> wrote:
>> > >>
>> > >> > On Mon, Dec 11, 2017 at 12:49 PM, Apekshit Sharma <
>> a...@cloudera.com>
>> > >> > wrote:
>> > >> >
>> > >> >> Seems like good idea:
>> > >> >> - remove long dead dependency
>> > >> >> - a bit cleaner code
>> > >> >> - hadoop also moved to slf4j
>> > >> >>
>> > >> >> Quickly looking at codebase to get idea of amount of work
>> required,
>> > >> here
>> > >> >> are some numbers:
>> > >> >> - LOG.debug : ~1800
>> > >> >> - LOG.trace : ~500
>> > >> >> - LOG.info: ~3000
>> > >> >>
>> > >> >> Looking at this patch (
>> > >> >> https://issues.apache.org/jira/secure/attachment/12901002/
>> > >> >> HBASE-19449.1.patch),
>> > >> >> seemed like tedious and repetitive task, was wondering if someone
>> has
>> > >> >> automated it already.
>> > >> >> Looks like this can help reduce a huge part of grunt work:
>> > >> >> https://www.slf4j.org/migrator.html.
>> > >> >>
>> > >> >> But before progressing, as a basic validation, can we see:
>> > >> >> - an example of old vs new log lines (that there is no diff, or we
>> > are
>> > >> >> comfortable with what's there)
>> > >> >> - an example of changes in properties file
>> > >> >>
>> > >> >> Maybe starting with hbase-examples module for quick POC.
>> > >> >>
>> > >> >>
>> > >> > I like your suggestion Appy,
>> > >> > S
>> > >> >
>> > >> >
>> > >> Balazs has a patch up on HBASE-10092 making the move. Intend to
>> commit
>> > it
>> > >> in next day or so unless objection.
>> > >> S
>> > >>
>> > >>
>> > >>
>> > >> >
>> > >> >
>> > >> >> -- Appy
>> > >> >>
>> > >> >
>> > >> >
>> > >>
>> > >
>> > >
>> > >
>> > > --
>> > >
>> > > -- Appy
>> > >
>> >
>> >
>> >
>> > --
>> >
>> > -- Appy
>> >
>>
>
>
