Uh oh!
There was an error while loading. Please reload this page.
HBASE-23210 Backport HBASE-15519 (Add per-user metrics) to branch-1 - #755
Merged
Conversation
apurtell
commented
Oct 24, 2019
ContributorAuthor
New unit tests pass |
apurtell
commented
Oct 24, 2019
ContributorAuthor
@joshelser I couldn't ask @ankitsinghal for a review for some reason, but i see you recently reviewed similar |
apurtellforce-pushed
the
HBASE-23210-branch-1
branch
from
October 24, 2019 22:47
b770177 to
b31f442Comparejoshelser
approved these changes
Oct 24, 2019
| class SweepRunnable implements Runnable { | ||
| @Override public void run() { | ||
| if (LOG.isTraceEnabled()) { | ||
| LOG.trace("Starting sweep of lossyCounting-" + name); |
Member
There was a problem hiding this comment.
nit, could use the {} markers and drop the isTraceEnabled()
ContributorAuthor
There was a problem hiding this comment.
This came in from branch-2 change. I agree but try not to second guess the original contributor on backports.
| try { | ||
| sweep(); | ||
| } catch (Exception exception) { | ||
| LOG.debug("Error while sweeping of lossyCounting-{}", name, exception); |
Member
There was a problem hiding this comment.
(damnit, lost the previous comment)
Make sure you're getting the right debug method here. The debug(String, Object, Exception) not debug(String, Object, Object). Hit this the other day.
apurtellforce-pushed
the
HBASE-23210-branch-1
branch
from
October 24, 2019 23:09
b31f442 to
ba2d5ffCompareApache-HBase
commented
Oct 25, 2019
💔 -1 overall
This message was automatically generated. |
apurtell
commented
Oct 25, 2019
ContributorAuthor
Back soon to fix checkstyle issues |
apurtellforce-pushed
the
HBASE-23210-branch-1
branch
from
October 25, 2019 04:25
ba2d5ff to
dc96c24Compareapurtell
commented
Oct 25, 2019
ContributorAuthor
Fixed some checkstyle nits, will merge tomorrow |
Apache-HBase
commented
Oct 25, 2019
💔 -1 overall
This message was automatically generated. |
Apache-HBase
commented
Oct 25, 2019
💔 -1 overall
This message was automatically generated. |
HBASE-15519 Add per-user metrics with lossy counting Introducing property hbase.regionserver.user.metrics.enabled(Default:true) to disable user metrics in case it accounts for any performance issues
apurtellforce-pushed
the
HBASE-23210-branch-1
branch
from
October 28, 2019 16:03
dc96c24 to
5127c2eCompareapurtell
commented
Oct 28, 2019
ContributorAuthor
Fixed the other checkstyle indent nit |
| this.data = new ConcurrentHashMap<>(); | ||
| this.listener = listener; | ||
| calculateCurrentTerm(); | ||
| executor = Executors.newSingleThreadExecutor(); |
Contributor
There was a problem hiding this comment.
I think this causes the RegionServer process to hang upon shutdown since we're not using daemon threads.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HBASE-15519 Add per-user metrics with lossy counting
Introducing property hbase.regionserver.user.metrics.enabled(Default:true)
to disable user metrics in case it accounts for any performance issues