stoty commented on code in PR #1552:
URL: https://github.com/apache/phoenix/pull/1552#discussion_r1133452385
##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/PhoenixMasterObserver.java:
##########
@@ -29,38 +29,51 @@ public Optional<MasterObserver> getMasterObserver() {
@Override
public void preMergeRegions(final
ObserverContext<MasterCoprocessorEnvironment> ctx, RegionInfo[] regionsToMerge)
throws IOException {
+ // proceed if it is a phoenix table
try {
- if (blockMergeForSaltedTables(ctx, regionsToMerge)) {
+ if (shouldBlockMerge(ctx, regionsToMerge)) {
//different salt so block merge
throw new IOException("Don't merge regions with different salt
bits");
}
} catch (SQLException e) {
+ // should we block here??
+ // this exception can be because of something wrong on cluster
+ // but to be on safe side we block merge here assuming blocking of
merge would have no impact
+ // but merging wrong regions can have data consistency issue
throw new IOException("SQLException while fetching data from
phoenix", e);
}
}
- private boolean
blockMergeForSaltedTables(ObserverContext<MasterCoprocessorEnvironment> ctx,
- RegionInfo[] regionsToMerge) throws SQLException {
- TableName table = regionsToMerge[0].getTable();
- int saltBuckets = getTableSalt(ctx, table.toString());
+ private boolean
shouldBlockMerge(ObserverContext<MasterCoprocessorEnvironment> ctx,
+ RegionInfo[] regionsToMerge) throws
SQLException {
+ TableName tableName = regionsToMerge[0].getTable();
+ PTable phoenixTable = getPhoenixTable(ctx, tableName);
+ if(phoenixTable == null) {
+ // it is not a phoenix table
+ return false;
+ }
+ int saltBuckets = phoenixTable.getBucketNum();
System.out.println("Number of buckets="+saltBuckets);
- if(saltBuckets > 0 ) {
- System.out.println("Number of buckets="+saltBuckets);
- return !regionsHaveSameSalt(regionsToMerge);
+ if(saltBuckets <= 0 ) {
Review Comment:
Nit: Having negative salt buckets here points to some serious problem with
the tabke.
I'd just check for 0 here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
