-
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-27398 Remove dumping of EOFException while reading WAL with ProtobufLogReader #4806
Conversation
💔 -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.
OK, after reading the related code, I do not think we should remove the dumping here.
We can throw EOFException at many places in the try block, and they all have a fat message to describe what is going on, and if we remove the dumping of EOFException below, there is no way for us to print out these messages.
IF this is too excessive, I think we should try to avoid throwing too many EOFExceptions for normal cases, for example, if we are tailing a WAL which is currently being written, then at the last position we will get an EOF, and if we want to catch up fast, we will get a lot EOF, which is expected, so we should try to reduce these noises, instead of purging the output for all the EOFException.
Thanks.
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@@ -446,7 +444,7 @@ protected boolean readNext(Entry entry) throws IOException { | |||
// Else restore our position to original location in hope that next time through we will | |||
// read successfully. | |||
LOG.debug("Encountered a malformed edit, seeking back to last good position in file, " | |||
+ "from {} to {}", inputStream.getPos(), originalPosition, eof); | |||
+ "from {} to {}", inputStream.getPos(), originalPosition); |
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.
@Apache9 how about this change (line 447)? This change can be done if not the one above where we are throwing EOF (line 430-431)?
This was -1ed by @Apache9 , so closing it |
No description provided.