This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch branch-3
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-3 by this push:
new 86ccf712cb5 HBASE-29706 Modify table with lazy mode should pass if
coprocessors have not changed (#7575)
86ccf712cb5 is described below
commit 86ccf712cb5f2ff3a4ee54fd58e415d618e67354
Author: Aman Poonia <[email protected]>
AuthorDate: Thu Jan 1 14:33:53 2026 +0530
HBASE-29706 Modify table with lazy mode should pass if coprocessors have
not changed (#7575)
Signed-off-by: Duo Zhang <[email protected]>
Reviewed-by: Umesh Kumar <[email protected]>
(cherry picked from commit a2177d787ccd8dac0986dfd94f204019a4745161)
---
.../hbase/client/CoprocessorDescriptorBuilder.java | 35 ++++++++--
.../master/procedure/ModifyTableProcedure.java | 26 +++++--
.../master/procedure/TestModifyTableProcedure.java | 79 ++++++++++++++++++++++
3 files changed, 131 insertions(+), 9 deletions(-)
diff --git
a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java
b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java
index cb0caca21b0..00d666e38d8 100644
---
a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java
+++
b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.client;
import java.util.Collections;
import java.util.Map;
+import java.util.NavigableMap;
import java.util.Objects;
import java.util.Optional;
import java.util.TreeMap;
@@ -42,7 +43,8 @@ public final class CoprocessorDescriptorBuilder {
private final String className;
private String jarPath;
private int priority = Coprocessor.PRIORITY_USER;
- private Map<String, String> properties = new TreeMap();
+ // HBASE-29706 Fix hashcode calculation. Use NavigableMap instead of Map
+ private NavigableMap<String, String> properties = new TreeMap<>();
public CoprocessorDescriptorBuilder setJarPath(String jarPath) {
this.jarPath = jarPath;
@@ -76,10 +78,10 @@ public final class CoprocessorDescriptorBuilder {
private final String className;
private final String jarPath;
private final int priority;
- private final Map<String, String> properties;
+ private final NavigableMap<String, String> properties;
private CoprocessorDescriptorImpl(String className, String jarPath, int
priority,
- Map<String, String> properties) {
+ NavigableMap<String, String> properties) {
this.className = className;
this.jarPath = jarPath;
this.priority = priority;
@@ -102,8 +104,8 @@ public final class CoprocessorDescriptorBuilder {
}
@Override
- public Map<String, String> getProperties() {
- return Collections.unmodifiableMap(properties);
+ public NavigableMap<String, String> getProperties() {
+ return Collections.unmodifiableNavigableMap(properties);
}
@Override
@@ -111,5 +113,28 @@ public final class CoprocessorDescriptorBuilder {
return "class:" + className + ", jarPath:" + jarPath + ", priority:" +
priority
+ ", properties:" + properties;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof CoprocessorDescriptor)) {
+ return false;
+ }
+ CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
+ if (
+ priority != other.getPriority() || !Objects.equals(className,
other.getClassName())
+ || !Objects.equals(getJarPath(), other.getJarPath())
+ ) {
+ return false;
+ }
+ return Objects.equals(getProperties(), other.getProperties());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(className, jarPath, priority, properties);
+ }
}
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java
index 0d8981891e5..3450f305910 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.master.procedure;
import java.io.IOException;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
@@ -32,6 +33,7 @@ import org.apache.hadoop.hbase.HBaseIOException;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.TableNotFoundException;
+import org.apache.hadoop.hbase.client.CoprocessorDescriptor;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionReplicaUtil;
import org.apache.hadoop.hbase.client.TableDescriptor;
@@ -132,10 +134,7 @@ public class ModifyTableProcedure extends
AbstractStateMachineTableProcedure<Mod
throw new HBaseIOException(
"Cannot add or remove column families when this modification " +
"won't reopen regions.");
}
- if (
- this.unmodifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
- !=
this.modifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
- ) {
+ if (isCoprocModified()) {
throw new HBaseIOException(
"Can not modify Coprocessor when table modification won't reopen
regions");
}
@@ -152,6 +151,25 @@ public class ModifyTableProcedure extends
AbstractStateMachineTableProcedure<Mod
}
}
+ private boolean isCoprocModified() {
+ final Collection<CoprocessorDescriptor> unmodifiedCoprocs =
+ this.unmodifiedTableDescriptor.getCoprocessorDescriptors();
+ final Collection<CoprocessorDescriptor> modifiedCoprocs =
+ this.modifiedTableDescriptor.getCoprocessorDescriptors();
+
+ if (unmodifiedCoprocs.size() != modifiedCoprocs.size()) {
+ return true;
+ }
+
+ final Set<CoprocessorDescriptor> unmodifiedSet = new
HashSet<>(unmodifiedCoprocs);
+ for (CoprocessorDescriptor cp : modifiedCoprocs) {
+ if (!unmodifiedSet.remove(cp)) {
+ return true;
+ }
+ }
+ return !unmodifiedSet.isEmpty();
+ }
+
/**
* Comparing the value associated with a given key across two
TableDescriptor instances'
* properties.
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java
index c8043b8ee3b..26b190f798b 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java
@@ -35,6 +35,8 @@ import
org.apache.hadoop.hbase.client.PerClientRandomNonceGenerator;
import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.TableDescriptor;
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
+import org.apache.hadoop.hbase.constraint.ConstraintProcessor;
+import org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver;
import org.apache.hadoop.hbase.io.compress.Compression;
import
org.apache.hadoop.hbase.master.procedure.MasterProcedureTestingUtility.StepHook;
import org.apache.hadoop.hbase.procedure2.Procedure;
@@ -698,4 +700,81 @@ public class TestModifyTableProcedure extends
TestTableDDLProcedureBase {
assertTrue(e.getMessage().contains("Can not modify REGION_REPLICATION"));
}
}
+
+ @Test
+ public void testModifyTableWithCoprocessorAndColumnFamilyPropertyChange()
throws IOException {
+ // HBASE-29706 - This test validates the fix for the bug where modifying
only column family
+ // properties
+ // (like COMPRESSION) with REOPEN_REGIONS=false would incorrectly throw an
error when
+ // coprocessors are present. The bug was caused by comparing collection
hash codes
+ // instead of actual descriptor content, which failed when HashMap
iteration order varied.
+
+ final boolean reopenRegions = false;
+ final TableName tableName = TableName.valueOf(name.getMethodName());
+ final ProcedureExecutor<MasterProcedureEnv> procExec =
getMasterProcedureExecutor();
+
+ MasterProcedureTestingUtility.createTable(procExec, tableName, null, "cf");
+
+ // Step 1: Add coprocessors to the table
+ TableDescriptor htd = UTIL.getAdmin().getDescriptor(tableName);
+ final String cp2 = ConstraintProcessor.class.getName();
+ TableDescriptor descriptorWithCoprocessor =
TableDescriptorBuilder.newBuilder(htd)
+
.setCoprocessor(CoprocessorDescriptorBuilder.newBuilder(SimpleRegionObserver.class.getName())
+ .setPriority(100).build())
+
.setCoprocessor(CoprocessorDescriptorBuilder.newBuilder(cp2).setPriority(200).build())
+ .build();
+ long procId = ProcedureTestingUtility.submitAndWait(procExec, new
ModifyTableProcedure(
+ procExec.getEnvironment(), descriptorWithCoprocessor, null, htd, false,
true));
+ ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId));
+
+ // Verify coprocessors were added
+ TableDescriptor currentHtd = UTIL.getAdmin().getDescriptor(tableName);
+ assertEquals(2, currentHtd.getCoprocessorDescriptors().size());
+ assertTrue("First coprocessor should be present",
+ currentHtd.hasCoprocessor(SimpleRegionObserver.class.getName()));
+ assertTrue("Second coprocessor should be present",
currentHtd.hasCoprocessor(cp2));
+
+ // Step 2: Modify only the column family property (COMPRESSION) with
REOPEN_REGIONS=false
+ // This should SUCCEED because we're not actually modifying the
coprocessor,
+ // just the column family compression setting.
+ htd = UTIL.getAdmin().getDescriptor(tableName);
+ TableDescriptor modifiedDescriptor =
+ TableDescriptorBuilder
+ .newBuilder(htd).modifyColumnFamily(ColumnFamilyDescriptorBuilder
+
.newBuilder("cf".getBytes()).setCompressionType(Compression.Algorithm.SNAPPY).build())
+ .build();
+
+ // This should NOT throw an error - the fix ensures order-independent
coprocessor comparison
+ long procId2 = ProcedureTestingUtility.submitAndWait(procExec, new
ModifyTableProcedure(
+ procExec.getEnvironment(), modifiedDescriptor, null, htd, false,
reopenRegions));
+ ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId2));
+
+ // Verify the modification succeeded
+ currentHtd = UTIL.getAdmin().getDescriptor(tableName);
+ assertEquals("Coprocessors should still be present", 2,
+ currentHtd.getCoprocessorDescriptors().size());
+ assertTrue("First coprocessor should still be present",
+ currentHtd.hasCoprocessor(SimpleRegionObserver.class.getName()));
+ assertTrue("Second coprocessor should still be present",
currentHtd.hasCoprocessor(cp2));
+ assertEquals("Compression should be updated in table descriptor",
Compression.Algorithm.SNAPPY,
+ currentHtd.getColumnFamily("cf".getBytes()).getCompressionType());
+
+ // Verify regions haven't picked up the change yet (since
reopenRegions=false)
+ for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
+ assertEquals("Regions should still have old compression",
Compression.Algorithm.NONE,
+
r.getTableDescriptor().getColumnFamily("cf".getBytes()).getCompressionType());
+ }
+
+ // Force regions to reopen
+ for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
+ getMaster().getAssignmentManager().move(r.getRegionInfo());
+ }
+
+ // After reopen, regions should have the new compression setting
+ for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
+ assertEquals("Regions should now have new compression after reopen",
+ Compression.Algorithm.SNAPPY,
+
r.getTableDescriptor().getColumnFamily("cf".getBytes()).getCompressionType());
+ }
+ }
}
