-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-25562 ReplicationSourceWALReader log and handle exception immediately without retrying #2943
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
Conversation
|
🎊 +1 overall
This message was automatically generated. |
wchevreuil
left a comment
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.
Yeah, no point in delay the EOF handling when _ eofAutoRecovery_ is on. Just a little remark on log verbosity when still retrying, maybe worth reduce log level to avoid flooding log with repetitive messages?
...c/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceWALReader.java
Outdated
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
saintstack
left a comment
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 other than @wchevreuil comment.
...c/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceWALReader.java
Outdated
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
| } catch (IOException e) { // stream related | ||
| if (!handleEofException(e)) { | ||
| LOG.error("Failed to read stream of replication entries", e); | ||
| } |
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.
The patch looks good to me too.
I had one non patch related question. Does it even make sense to sleep if we were able to handle eof exception successfully ?
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.
Nice question! We should skip sleep if the exception can be handled. I modified it.
Thanks for reviwing.
|
🎊 +1 overall
This message was automatically generated. |
shahrs87
left a comment
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.
+1 ltgm. Thank you !
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…iately without retrying
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…iately without retrying (#2943) Signed-off-by: Wellington Chevreuil <[email protected]> Signed-off-by: stack <[email protected]> Signed-off-by: shahrs87
…iately without retrying (#2943) Signed-off-by: Wellington Chevreuil <[email protected]> Signed-off-by: stack <[email protected]> Signed-off-by: shahrs87
…iately without retrying (apache#2943) Signed-off-by: Wellington Chevreuil <[email protected]> Signed-off-by: stack <[email protected]> Signed-off-by: shahrs87
…iately without retrying (apache#2943) Signed-off-by: Wellington Chevreuil <[email protected]> Signed-off-by: stack <[email protected]> Signed-off-by: shahrs87
…iately without retrying