Skip to content

Commit cd74717

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

File tree

3 files changed

+97
-2
lines changed

3 files changed

+97
-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: 72 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,75 @@ 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 properties
706+
// (like COMPRESSION) with REOPEN_REGIONS=false would incorrectly throw an error when
707+
// coprocessors are present. The bug was caused by comparing collection hash codes
708+
// instead of actual descriptor content, which failed when HashMap iteration order varied.
709+
710+
final boolean reopenRegions = false;
711+
final TableName tableName = TableName.valueOf(name.getMethodName());
712+
final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
713+
714+
MasterProcedureTestingUtility.createTable(procExec, tableName, null, "cf");
715+
716+
// Step 1: Add a coprocessor to the table
717+
TableDescriptor htd = UTIL.getAdmin().getDescriptor(tableName);
718+
TableDescriptor descriptorWithCoprocessor = TableDescriptorBuilder.newBuilder(htd)
719+
.setCoprocessor(CoprocessorDescriptorBuilder
720+
.newBuilder(SimpleRegionObserver.class.getName())
721+
.setPriority(100)
722+
.build())
723+
.build();
724+
long procId = ProcedureTestingUtility.submitAndWait(procExec, new ModifyTableProcedure(
725+
procExec.getEnvironment(), descriptorWithCoprocessor, null, htd, false, true));
726+
ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId));
727+
728+
// Verify coprocessor was added
729+
TableDescriptor currentHtd = UTIL.getAdmin().getDescriptor(tableName);
730+
assertEquals(1, currentHtd.getCoprocessorDescriptors().size());
731+
732+
// Step 2: Modify only the column family property (COMPRESSION) with REOPEN_REGIONS=false
733+
// This should SUCCEED because we're not actually modifying the coprocessor,
734+
// just the column family compression setting.
735+
htd = UTIL.getAdmin().getDescriptor(tableName);
736+
TableDescriptor modifiedDescriptor = TableDescriptorBuilder.newBuilder(htd)
737+
.modifyColumnFamily(ColumnFamilyDescriptorBuilder
738+
.newBuilder("cf".getBytes())
739+
.setCompressionType(Compression.Algorithm.SNAPPY)
740+
.build())
741+
.build();
742+
743+
// This should NOT throw an error - the fix ensures order-independent coprocessor comparison
744+
long procId2 = ProcedureTestingUtility.submitAndWait(procExec, new ModifyTableProcedure(
745+
procExec.getEnvironment(), modifiedDescriptor, null, htd, false, reopenRegions));
746+
ProcedureTestingUtility.assertProcNotFailed(procExec.getResult(procId2));
747+
748+
// Verify the modification succeeded
749+
currentHtd = UTIL.getAdmin().getDescriptor(tableName);
750+
assertEquals("Coprocessor should still be present", 1,
751+
currentHtd.getCoprocessorDescriptors().size());
752+
assertEquals("Compression should be updated in table descriptor", Compression.Algorithm.SNAPPY,
753+
currentHtd.getColumnFamily("cf".getBytes()).getCompressionType());
754+
755+
// Verify regions haven't picked up the change yet (since reopenRegions=false)
756+
for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
757+
assertEquals("Regions should still have old compression", Compression.Algorithm.NONE,
758+
r.getTableDescriptor().getColumnFamily("cf".getBytes()).getCompressionType());
759+
}
760+
761+
// Force regions to reopen
762+
for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
763+
getMaster().getAssignmentManager().move(r.getRegionInfo());
764+
}
765+
766+
// After reopen, regions should have the new compression setting
767+
for (HRegion r : UTIL.getHBaseCluster().getRegions(tableName)) {
768+
assertEquals("Regions should now have new compression after reopen",
769+
Compression.Algorithm.SNAPPY,
770+
r.getTableDescriptor().getColumnFamily("cf".getBytes()).getCompressionType());
771+
}
772+
}
701773
}

0 commit comments

Comments
 (0)