-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-21406 "status 'replication'" should not show SINK if the cluste… #1761
Conversation
…r does not act as sink
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Left a couple of nits.
assertEquals(loadSink.getTimestampStarted(), loadSink.getTimestampsOfLastAppliedOp()); | ||
//now insert some rows on source, so that it gets delivered to target | ||
insertRowsOnSource(); | ||
Thread.sleep(10000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although 10s seems more than enough time, we can use Waiter.wait()
/ HBASE_TESTING_UTILITY.waitFor()
with predicate to check if loadSink.getTimestampsOfLastAppliedOp()>loadSink.getTimestampStarted()
.
} | ||
|
||
/** | ||
* Gets the total number of OPs delivered to target by this sink. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Gets the total number of OPs delivered to this sink
would be better? WDYT?
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit, +1 otherwise
//now insert some rows on source, so that it gets delivered to target | ||
insertRowsOnSource(); | ||
long wait = Waiter.waitFor(UTIL2.getConfiguration(), | ||
100000, new Waiter.Predicate<Exception>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to keep it 10s instead of 100s? :)
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Left a minor NIT. Are test failures related?
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsSink.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one minor feedback in the ruby code -- fine to fix on commit.
Looks good!
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
apache#1761) Signed-off-by: Jan Hentschel <jan.hentschel@ultratendency.com> Signed-off by: Viraj Jasani <vjasani@apache.org> Signed-off-by: Josh Elser <elserj@apache.org> (Cherry picked from commit e5345b3)
…r does not act as sink