-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-25766 Introduce RegionSplitRestriction that restricts the … #3150
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
89fe8fb
HBASE-25766 Introduce RegionSplitPointRestriction that restricts the …
brfrn169 c5b26ed
Fix checkstyle errors
brfrn169 c7f0cd6
Some small modifications based on the reviews
brfrn169 6de6908
Rename RegionSplitPointRestriction to RegionSplitRestriction
brfrn169 b6006d7
Fix the checkstyle error
brfrn169 c6bfe5d
Add some JavaDoc for RegionSplitRestriction
brfrn169 819e25a
Add a WARN message and some JavaDoc
brfrn169 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
84 changes: 84 additions & 0 deletions
84
...n/java/org/apache/hadoop/hbase/regionserver/DelimitedKeyPrefixRegionSplitRestriction.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.hadoop.hbase.regionserver; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Arrays; | ||
| import org.apache.hadoop.conf.Configuration; | ||
| import org.apache.hadoop.hbase.client.TableDescriptor; | ||
| import org.apache.hadoop.hbase.util.Bytes; | ||
| import org.apache.yetus.audience.InterfaceAudience; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** | ||
| * A {@link RegionSplitRestriction} implementation that groups rows by a prefix of the row-key with | ||
| * a delimiter. Only the first delimiter for the row key will define the prefix of the row key that | ||
| * is used for grouping. | ||
| * <p> | ||
| * This ensures that a region is not split "inside" a prefix of a row key. | ||
| * I.e. rows can be co-located in a region by their prefix. | ||
| * | ||
| * As an example, if you have row keys delimited with <code>_</code>, like | ||
| * <code>userid_eventtype_eventid</code>, and use prefix delimiter _, this split policy ensures | ||
| * that all rows starting with the same userid, belongs to the same region. | ||
| */ | ||
| @InterfaceAudience.Private | ||
| public class DelimitedKeyPrefixRegionSplitRestriction extends RegionSplitRestriction { | ||
| private static final Logger LOG = | ||
| LoggerFactory.getLogger(DelimitedKeyPrefixRegionSplitRestriction.class); | ||
|
|
||
| public static final String DELIMITER_KEY = | ||
| "hbase.regionserver.region.split_restriction.delimiter"; | ||
|
|
||
| private byte[] delimiter = null; | ||
|
|
||
| @Override | ||
| public void initialize(TableDescriptor tableDescriptor, Configuration conf) throws IOException { | ||
| String delimiterString = tableDescriptor.getValue(DELIMITER_KEY); | ||
| if (delimiterString == null || delimiterString.length() == 0) { | ||
| delimiterString = conf.get(DELIMITER_KEY); | ||
| if (delimiterString == null || delimiterString.length() == 0) { | ||
| LOG.error("{} not specified for table {}. " | ||
| + "Using the default RegionSplitRestriction", DELIMITER_KEY, | ||
| tableDescriptor.getTableName()); | ||
| return; | ||
| } | ||
| } | ||
| delimiter = Bytes.toBytes(delimiterString); | ||
| } | ||
|
|
||
| @Override | ||
| public byte[] getRestrictedSplitPoint(byte[] splitPoint) { | ||
| if (delimiter != null) { | ||
| // find the first occurrence of delimiter in split point | ||
| int index = org.apache.hbase.thirdparty.com.google.common.primitives.Bytes.indexOf( | ||
| splitPoint, delimiter); | ||
| if (index < 0) { | ||
| LOG.warn("Delimiter {} not found for split key {}", Bytes.toString(delimiter), | ||
| Bytes.toStringBinary(splitPoint)); | ||
| return splitPoint; | ||
| } | ||
|
|
||
| // group split keys by a prefix | ||
| return Arrays.copyOf(splitPoint, Math.min(index, splitPoint.length)); | ||
| } else { | ||
| return splitPoint; | ||
| } | ||
| } | ||
| } |
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
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
76 changes: 76 additions & 0 deletions
76
...r/src/main/java/org/apache/hadoop/hbase/regionserver/KeyPrefixRegionSplitRestriction.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.hadoop.hbase.regionserver; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Arrays; | ||
| import org.apache.hadoop.conf.Configuration; | ||
| import org.apache.hadoop.hbase.client.TableDescriptor; | ||
| import org.apache.yetus.audience.InterfaceAudience; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| /** | ||
| * A {@link RegionSplitRestriction} implementation that groups rows by a prefix of the row-key. | ||
| * <p> | ||
| * This ensures that a region is not split "inside" a prefix of a row key. | ||
| * I.e. rows can be co-located in a region by their prefix. | ||
| */ | ||
| @InterfaceAudience.Private | ||
| public class KeyPrefixRegionSplitRestriction extends RegionSplitRestriction { | ||
| private static final Logger LOG = | ||
| LoggerFactory.getLogger(KeyPrefixRegionSplitRestriction.class); | ||
|
|
||
| public static final String PREFIX_LENGTH_KEY = | ||
| "hbase.regionserver.region.split_restriction.prefix_length"; | ||
|
|
||
| private int prefixLength; | ||
|
|
||
| @Override | ||
| public void initialize(TableDescriptor tableDescriptor, Configuration conf) throws IOException { | ||
| String prefixLengthString = tableDescriptor.getValue(PREFIX_LENGTH_KEY); | ||
| if (prefixLengthString == null) { | ||
| prefixLengthString = conf.get(PREFIX_LENGTH_KEY); | ||
| if (prefixLengthString == null) { | ||
| LOG.error("{} not specified for table {}. " | ||
| + "Using the default RegionSplitRestriction", PREFIX_LENGTH_KEY, | ||
| tableDescriptor.getTableName()); | ||
| return; | ||
| } | ||
| } | ||
| try { | ||
| prefixLength = Integer.parseInt(prefixLengthString); | ||
| } catch (NumberFormatException ignored) { | ||
| } | ||
| if (prefixLength <= 0) { | ||
| LOG.error("Invalid value for {} for table {}:{}. " | ||
| + "Using the default RegionSplitRestriction", PREFIX_LENGTH_KEY, | ||
| tableDescriptor.getTableName(), prefixLengthString); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public byte[] getRestrictedSplitPoint(byte[] splitPoint) { | ||
| if (prefixLength > 0) { | ||
| // group split keys by a prefix | ||
| return Arrays.copyOf(splitPoint, Math.min(prefixLength, splitPoint.length)); | ||
| } else { | ||
| return splitPoint; | ||
| } | ||
| } | ||
| } |
40 changes: 40 additions & 0 deletions
40
...e-server/src/main/java/org/apache/hadoop/hbase/regionserver/NoRegionSplitRestriction.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.hadoop.hbase.regionserver; | ||
|
|
||
| import java.io.IOException; | ||
| import org.apache.hadoop.conf.Configuration; | ||
| import org.apache.hadoop.hbase.client.TableDescriptor; | ||
| import org.apache.yetus.audience.InterfaceAudience; | ||
|
|
||
| /** | ||
| * A {@link RegionSplitRestriction} implementation that does nothing. | ||
| */ | ||
| @InterfaceAudience.Private | ||
| public class NoRegionSplitRestriction extends RegionSplitRestriction { | ||
|
|
||
| @Override | ||
| public void initialize(TableDescriptor tableDescriptor, Configuration conf) throws IOException { | ||
| } | ||
|
|
||
| @Override | ||
| public byte[] getRestrictedSplitPoint(byte[] splitPoint) { | ||
| // Do nothing | ||
| return splitPoint; | ||
| } | ||
| } |
Oops, something went wrong.
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.
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.
We do not have this logic in the past? What is changed so now we need to apply this restriction in SplitTableRegionProcedure? IIRC the logic is done at region server side?
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.
Yes, we didn't have this logic in the past. I think we can apply the restriction to a user-specified split point because without this logic, we can easily break the restriction by splitting with specifying a split point. And the user-specified split point is passed to the Master side, we need to do it on the master side.
What do you think? @Apache9
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.
Finally we should get the actual split point back from region server? No? Then this should be a bug for the current code base?
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.
No, I don't think so.
Let's see we have a table that has a key prefix restriction where the prefix length is 2 bytes.
When a user runs split command with specifying a split point
abcin the hbase shell, this will break the key prefix restriction if we split the region byabc. So I think we can apply the restriction to the user-specified split point, and the restriction-applied split point will beab, which won't break the restriction.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 What about this? Thanks.
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.
Good point. So user may be 'surprised' if we do not split where they want? Will there be a message saying so anywhere that their choice has been over-ruled by the restriction? Or will it be obvious that the 'restriction' over-ruled?
I'm good w/ the restriction over-ruling the user as long as there a log to this effect (add the 'behavior change' to the existing nice release note @brfrn169 )
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.
I think we can add a WARN message in the Master log when the user-specified split point is over-ruled by the restriction. I will do that. And I will add the 'behavior change' to the release note. Thanks.
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.
So here we will only use SplitRestriction to fix the split row? Then what if users uses the deprecated KeyPrefixSplitPolicy? We will not fix the split row if it breaks the rule?
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.
Yes, that's the behavior of KeyPrefixSplitPolicy, and we will not fix the split row even if it breaks the rule. And maybe it's not easy to fix it because RegionSplitPolicy doesn't have any method to restrict/convert a user-specified split point. It has only
byte[] getSplitPoint()that gets an appropriate split point calculated based on its policy.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, so in fact we are not changing the behavior? If you use the old KeyPrefixSplitPolicy, there is nothing changed. If you use the new SplitRestriction, then you will find out that you are not allowed to break the restriction when proposing a split point. Could mention this in the release note.