Skip to content

Commit 4272f2c

Browse files
committed
HBASE-29706: Make CoprocessorDescriptorImpl.equals() and hashCode() robust
1 parent 1c2025c commit 4272f2c

File tree

3 files changed

+94
-2
lines changed

3 files changed

+94
-2
lines changed

hbase-client/src/main/java/org/apache/hadoop/hbase/client/CoprocessorDescriptorBuilder.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,5 +111,28 @@ public String toString() {
111111
return "class:" + className + ", jarPath:" + jarPath + ", priority:" + priority
112112
+ ", properties:" + properties;
113113
}
114+
115+
@Override
116+
public boolean equals(Object obj) {
117+
if (this == obj) {
118+
return true;
119+
}
120+
if (obj == null || !(obj instanceof CoprocessorDescriptor)) {
121+
return false;
122+
}
123+
CoprocessorDescriptor other = (CoprocessorDescriptor) obj;
124+
if (
125+
priority != other.getPriority() || !Objects.equals(className, other.getClassName())
126+
|| !Objects.equals(getJarPath(), other.getJarPath())
127+
) {
128+
return false;
129+
}
130+
return new TreeMap<>(getProperties()).equals(new TreeMap<>(other.getProperties()));
131+
}
132+
133+
@Override
134+
public int hashCode() {
135+
return Objects.hash(className, jarPath, priority, new TreeMap<>(properties));
136+
}
114137
}
115138
}

hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ protected void preflightChecks(MasterProcedureEnv env, Boolean enabled) throws H
133133
"Cannot add or remove column families when this modification " + "won't reopen regions.");
134134
}
135135
if (
136-
this.unmodifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
137-
!= this.modifiedTableDescriptor.getCoprocessorDescriptors().hashCode()
136+
!new HashSet<>(this.unmodifiedTableDescriptor.getCoprocessorDescriptors())
137+
.equals(new HashSet<>(this.modifiedTableDescriptor.getCoprocessorDescriptors()))
138138
) {
139139
throw new HBaseIOException(
140140
"Can not modify Coprocessor when table modification won't reopen regions");

hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestModifyTableProcedure.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.apache.hadoop.hbase.client.RegionInfo;
3636
import org.apache.hadoop.hbase.client.TableDescriptor;
3737
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
38+
import org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver;
3839
import org.apache.hadoop.hbase.io.compress.Compression;
3940
import org.apache.hadoop.hbase.master.procedure.MasterProcedureTestingUtility.StepHook;
4041
import org.apache.hadoop.hbase.procedure2.Procedure;
@@ -698,4 +699,72 @@ public void testModifyWillNotReopenRegions() throws IOException {
698699
assertTrue(e.getMessage().contains("Can not modify REGION_REPLICATION"));
699700
}
700701
}
702+
703+
@Test
704+
public void testModifyTableWithCoprocessorAndColumnFamilyPropertyChange() throws IOException {
705+
// HBASE-29706 - This test validates the fix for the bug where modifying only column family
706+
// properties
707+
// (like COMPRESSION) with REOPEN_REGIONS=false would incorrectly throw an error when
708+
// coprocessors are present. The bug was caused by comparing collection hash codes
709+
// instead of actual descriptor content, which failed when HashMap iteration order varied.
710+
711+
final boolean reopenRegions = false;
712+
final TableName tableName = TableName.valueOf(name.getMethodName());
713+
final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
714+
715+
MasterProcedureTestingUtility.createTable(procExec, tableName, null, "cf");
716+
717+
// Step 1: Add a coprocessor to the table
718+
TableDescriptor htd = UTIL.getAdmin().getDescriptor(tableName);
719+
TableDescriptor descriptorWithCoprocessor =
720+
TableDescriptorBuilder.newBuilder(htd).setCoprocessor(CoprocessorDescriptorBuilder
721+
.newBuilder(SimpleRegionObserver.class.getName()).setPriority(100).build()).build();
722+
long procId = ProcedureTestingUtility.submitAndWait(procExec, new ModifyTableProcedure(
723+
procExec.getEnvironment(), descriptorWithCoprocessor, null, htd, false, true));
724+
ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId));
725+
726+
// Verify coprocessor was added
727+
TableDescriptor currentHtd = UTIL.getAdmin().getDescriptor(tableName);
728+
assertEquals(1, currentHtd.getCoprocessorDescriptors().size());
729+
730+
// Step 2: Modify only the column family property (COMPRESSION) with REOPEN_REGIONS=false
731+
// This should SUCCEED because we're not actually modifying the coprocessor,
732+
// just the column family compression setting.
733+
htd = UTIL.getAdmin().getDescriptor(tableName);
734+
TableDescriptor modifiedDescriptor =
735+
TableDescriptorBuilder
736+
.newBuilder(htd).modifyColumnFamily(ColumnFamilyDescriptorBuilder
737+
.newBuilder("cf".getBytes()).setCompressionType(Compression.Algorithm.SNAPPY).build())
738+
.build();
739+
740+
// This should NOT throw an error - the fix ensures order-independent coprocessor comparison
741+
long procId2 = ProcedureTestingUtility.submitAndWait(procExec, new ModifyTableProcedure(
742+
procExec.getEnvironment(), modifiedDescriptor, null, htd, false, reopenRegions));
743+
ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId2));
744+
745+
// Verify the modification succeeded
746+
currentHtd = UTIL.getAdmin().getDescriptor(tableName);
747+
assertEquals("Coprocessor should still be present", 1,
748+
currentHtd.getCoprocessorDescriptors().size());
749+
assertEquals("Compression should be updated in table descriptor", Compression.Algorithm.SNAPPY,
750+
currentHtd.getColumnFamily("cf".getBytes()).getCompressionType());
751+
752+
// Verify regions haven't picked up the change yet (since reopenRegions=false)
753+
for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
754+
assertEquals("Regions should still have old compression", Compression.Algorithm.NONE,
755+
r.getTableDescriptor().getColumnFamily("cf".getBytes()).getCompressionType());
756+
}
757+
758+
// Force regions to reopen
759+
for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
760+
getMaster().getAssignmentManager().move(r.getRegionInfo());
761+
}
762+
763+
// After reopen, regions should have the new compression setting
764+
for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
765+
assertEquals("Regions should now have new compression after reopen",
766+
Compression.Algorithm.SNAPPY,
767+
r.getTableDescriptor().getColumnFamily("cf".getBytes()).getCompressionType());
768+
}
769+
}
701770
}

0 commit comments

Comments
 (0)