Uh oh!
There was an error while loading. Please reload this page.
HBASE-27223 Avoid data inconsistent between primary and secondary rep… - #4633
Closed
comnetwork wants to merge 2 commits into
Closed
HBASE-27223 Avoid data inconsistent between primary and secondary rep…#4633comnetwork wants to merge 2 commits into
comnetwork wants to merge 2 commits into
Conversation
…licas for the new region replication framework
Apache-HBase
commented
Jul 19, 2022
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Jul 19, 2022
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Jul 19, 2022
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Jul 19, 2022
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Jul 19, 2022
🎊 +1 overall
This message was automatically generated. |
Apache-HBase
commented
Jul 19, 2022
🎊 +1 overall
This message was automatically generated. |
Apache9
reviewed
Jul 20, 2022
| protected static final int DEFAULT_SLOW_SYNC_ROLL_INTERVAL_MS = 60 * 1000; // in ms, 1 minute | ||
| protected static final String WAL_SYNC_TIMEOUT_MS = "hbase.regionserver.wal.sync.timeout"; | ||
| public static final String WAL_SYNC_TIMEOUT_MS = "hbase.regionserver.wal.sync.timeout"; |
Contributor
There was a problem hiding this comment.
This is a missing part in our design, usually, if here we get a timeout exception, the only correct way is to abort the region server, as the design of WAL sync, is to succeed or die, there is no 'failure'. It is usually not a big deal is because we set a very large default value here, 5 minutes, usually the WAL system will abort the region server if it can not finish the sync within 5 minutes...
So I think we should throw a special IOException to upper layer, if we get this exception, we abort the region server.
ContributorAuthor
There was a problem hiding this comment.
@Apache9 ,ok, thank you very much for explanation, I would try to fix the code following your point.
Contributor
There was a problem hiding this comment.
I think this should be another issue.
ContributorAuthor
There was a problem hiding this comment.
@Apache9 , after I read the code about WAL , I have a question:
For AsyncFSWAL, basically WAL.sync could not throw any exception except that TimeoutIOException,
but for FSHLog, WAL.sync could throw any exception thrown by ProtobufLogWriter.append andProtobufLogWriter.sync , and when throwing these exceptions, it just requests the WAL rolling and does not abort the RegionServer, so for AsyncFSWAL,we could abort the RegionServer, but for FSHLog, it is not suitable to abort the RegionServer when WAL.sync throws the exception other than TimeoutIOException , we still need a way to avoid the situation described by this issue.
Contributor
There was a problem hiding this comment.
This is also a problem for the FSHLog implementation. Basically, if the write to HDFS fails, we do not know whether the data has been persistent or not. The implementation for AsyncFSWAL, is to open a new writer and try to write the WAL entries again, and then adding logic in WAL split and replay to deal with duplicate entries. So for FSHLog, if it is not easy to add the same logic with AsyncFSWAL, the correct way is to abort the region server to let the failover logic to detect whether the WAL entries have been persistent or not.
ContributorAuthor
There was a problem hiding this comment.
@Apache9, ok, I think we could open two new jiras , one is for AsyncFSWAL to abort the RegionServer for TimeoutIOException, and the other is to implement the retry WAL entries logic of HLog as same as AsyncFSWAL , which is a little more complicated.
ContributorAuthor
There was a problem hiding this comment.
@Apache9 , I have opened HBASE-27230 and HBASE-27231 for these two problems.
This was referenced Aug 23, 2022
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
3 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.
…licas for the new region replication framework