-
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-25562 ReplicationSourceWALReader log and handle exception immediately without retrying #2943
Conversation
🎊 +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.
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. |
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. |
@@ -148,12 +148,11 @@ public void run() { | |||
} | |||
} | |||
} 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. |
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 <wchevreuil@apache.org> Signed-off-by: stack <stack@apache.org> Signed-off-by: shahrs87
…iately without retrying (#2943) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Signed-off-by: stack <stack@apache.org> Signed-off-by: shahrs87
…iately without retrying (apache#2943) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Signed-off-by: stack <stack@apache.org> Signed-off-by: shahrs87
…iately without retrying (apache#2943) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Signed-off-by: stack <stack@apache.org> Signed-off-by: shahrs87
…iately without retrying