HBASE-24632 Enable procedure-based log splitting as default in hbase3 #2006
Conversation
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
|
🎊 +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. |
Contributor
|
+1 |
infraio
approved these changes
Jul 1, 2020
|
💔 -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. |
virajjasani
reviewed
Jul 3, 2020
Contributor
virajjasani
left a comment
virajjasani
left a comment
There was a problem hiding this comment.
Changes are good but PR destination should be master?
| * @deprecated since 2.4.0 and in 3.0.0, to be removed in 4.0.0. | ||
| */ | ||
| @Deprecated | ||
| public static final boolean DEFAULT_HBASE_SPLIT_COORDINATED_BY_ZK = false; |
Contributor
There was a problem hiding this comment.
We want this change only in master branch and not in branch-2 right?
Contributor
Author
There was a problem hiding this comment.
Dunno. I think we should enable it for 2.4... let me say so up on the JIRA and see what folks say.
Contributor
|
We have some WAL test failures with this setting. |
Contributor
Author
|
Test faillures are because we try to delete a non-empty directory. The left-over WALs are meta WALs but for meta regions that have since moved to another server (after a successful close); i.e. they WALs are no longer needed... They are for archive. The old zk-based WAL splitter specifically handled this case archiving remaining meta files if the crashed server was NOT carrying meta. Added this special handling to the new procedure-based WAL split which was missing it. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
anoopsjohn
reviewed
Jul 7, 2020
| * the ServerCrashProcedure won't split these leftover hbase:meta WALs, just leaving them in | ||
| * the WAL splitting dir. If we try to delete the WAL splitting for the server, it fail since | ||
| * the dir is not totally empty. We can safely archive these hbase:meta log; then the | ||
| * WAL dir can be deleted. |
Contributor
There was a problem hiding this comment.
Ya this was an issue in zk based split also and later fixed I guess. One Q. When the META region was moved to another RS, the meta wal file would have been closed right? Later the file would have been archived by the chore. This is how normal wal files will get archived. So is that not at all happening for META wals? Or it is happening but in test, the Chore did not get a chance to archive it yet.
This fix is needed anyways. Am trying to see do we have another issue also
Contributor
Author
There was a problem hiding this comment.
Makes sense Anoop. I think its just that its there when the split wal finishes... It is quiet possible chore has not run yet. Indeed the meta has to have closed cleanly.
|
💔 -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. |
Contributor
Author
|
We weren't handling corrupt WALs |
|
💔 -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. |
… Add deprecation of 'classic' zk-based WAL splitter. Also fix three bugs: * We were trying to delete non-empty directory; weren't doing accounting for meta WALs where meta had moved off the server (successfully) * We were deleting split WALs rather than archiving them. * We were not handling corrupt files. Deprecations and removal of tests of old system.
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
|
Merged manually |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 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.
No description provided.