Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP #1316 Lifecycle logging #1372

Closed
wants to merge 2 commits into from

Conversation

keith-turner
Copy link
Contributor

I did some experimentation of what the structure for #1316 might look like. Sharing this experimentation for discussion only. For this PR I put the logging code in a single package intentionally. The reason for this is it would make it easy for user to look at the source code for this single package and see the important log messages Accumulo generates. Having it all in one place may also make it easier to generate a grok file for logstash, making a task like apache/fluo-muchos#254 easier.

I gave a lot of thought to the logical logger hierarchy, but its very incomplete.

@ctubbsii ctubbsii changed the title WIP #1316 WIP #1316 Lifecycle logging Sep 23, 2019
Moved logging of Accumulo metadata from tablet to its own logger.  This
allows logging of metadata to be turned on/off independantly from tablet
lifecycle events.
@EdColeman
Copy link
Contributor

From a quick review, I like the approach.

Do you have any thoughts concerning a ZooKeeper logger - used in ZooReader, ZooStore,... that could log zookeeper operations - or would that be too low level and best just done with current debug logs?

@hkeebler
Copy link
Contributor

I like the tidyness and uniformity. Logging can get messy but there is a loss of whether something is logged to trace, or debug, or error. Maybe it could be reflected in the method call like logger.traceTabletMutated(...) . And, I have to admit since @Manno15 and I are trying to understand the migration process for a ticket he is working being able to follow the "exact" log message back into the code is very helpful.

@keith-turner
Copy link
Contributor Author

Do you have any thoughts concerning a ZooKeeper logger

I might be in favor of that. The only concern I would have is I would not want to duplicate any functionality the zookeeper client offers in terms of logging. Also, would it be better to change zookeeper to have more useful logging rather than Accumulo?

follow the "exact" log message back into the code is very helpful.

It is interesting that this code could become an accidental guide book into the most important parts of the Accumulo code. One that is always kept up to date.

@keith-turner
Copy link
Contributor Author

This PR was created for feedback. Took a first step in implementing this in #1480

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants