This is an automated email from the ASF dual-hosted git repository.
liuxiaocs7 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 ae06e5308c3 HBASE-30246 MOB compaction does not close MobCell after
resolving ref… (#8392)
ae06e5308c3 is described below
commit ae06e5308c38acd50e937ccacf2545e4c9417f4f
Author: Xiao Liu <[email protected]>
AuthorDate: Fri Jun 26 12:37:51 2026 +0800
HBASE-30246 MOB compaction does not close MobCell after resolving ref…
(#8392)
Signed-off-by: Peng Lu <[email protected]>
---
.../hadoop/hbase/mob/DefaultMobStoreCompactor.java | 26 ++++++++++++++++++++-
.../hadoop/hbase/mob/FaultyMobStoreCompactor.java | 4 ++--
.../hbase/mob/TestDefaultMobStoreCompactor.java | 27 ++++++++++++++++++++++
.../hadoop/hbase/regionserver/TestHMobStore.java | 19 +++++++++------
4 files changed, 66 insertions(+), 10 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
index cc38fac5a15..e9c1c62dbbc 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java
@@ -37,6 +37,7 @@ import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.ExtendedCell;
import org.apache.hadoop.hbase.KeyValue;
+import org.apache.hadoop.hbase.KeyValueUtil;
import org.apache.hadoop.hbase.PrivateCellUtil;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.regionserver.CellSink;
@@ -174,6 +175,29 @@ public class DefaultMobStoreCompactor extends
DefaultCompactor {
MobConstants.DEFAULT_MOB_COMPACTION_READ_CACHE_BLOCKS);
}
+ /**
+ * Resolves a MOB reference cell to its backing MOB value and returns an
independent,
+ * heap-resident copy of the resolved cell.
+ * <p>
+ * A MOB cell resolved from a MOB file is backed by a {@code
StoreFileScanner}; closing the
+ * {@link MobCell} closes that scanner and may release/recycle the NIO
buffers referenced by the
+ * returned cell. We close the {@link MobCell} here to avoid leaking
scanners/buffers while
+ * compacting many reference cells.
+ * <p>
+ * The {@link KeyValueUtil#copyToNewKeyValue(ExtendedCell)} call is required
by this ownership
+ * model: HFile writers and encoders may retain references to appended cells
(e.g.
+ * {@code lastCell}, {@code firstCellInBlock}, and the data block encoder's
{@code prevCell})
+ * until {@code beforeShipped()}. Returning the scanner-backed cell directly
would let those later
+ * reads access released buffers. Removing this copy would require changing
the caller to retain
+ * each {@link MobCell} and close it only after the writers have shipped
their retained
+ * references.
+ */
+ protected ExtendedCell resolveMobCell(ExtendedCell reference) throws
IOException {
+ try (MobCell mobCell = mobStore.resolve(reference,
cacheMobBlocksOnCompaction, false)) {
+ return KeyValueUtil.copyToNewKeyValue(mobCell.getCell());
+ }
+ }
+
@Override
public List<Path> compact(CompactionRequestImpl request,
ThroughputController throughputController, User user) throws IOException {
@@ -379,7 +403,7 @@ public class DefaultMobStoreCompactor extends
DefaultCompactor {
String fName = MobUtils.getMobFileName(c);
// Added to support migration
try {
- mobCell = mobStore.resolve(c, cacheMobBlocksOnCompaction,
false).getCell();
+ mobCell = resolveMobCell(c);
} catch (DoNotRetryIOException e) {
if (
discardMobMiss && e.getCause() != null
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/FaultyMobStoreCompactor.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/FaultyMobStoreCompactor.java
index 536f40eb0af..bdb41d40410 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/FaultyMobStoreCompactor.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/FaultyMobStoreCompactor.java
@@ -199,7 +199,7 @@ public class FaultyMobStoreCompactor extends
DefaultMobStoreCompactor {
String fName = MobUtils.getMobFileName(c);
// Added to support migration
try {
- mobCell = mobStore.resolve(c, cacheMobBlocksOnCompaction,
false).getCell();
+ mobCell = resolveMobCell(c);
} catch (DoNotRetryIOException e) {
if (
discardMobMiss && e.getCause() != null
@@ -269,7 +269,7 @@ public class FaultyMobStoreCompactor extends
DefaultMobStoreCompactor {
} else {
// If the value is not larger than the threshold, it's not
regarded a mob. Retrieve
// the mob cell from the mob file, and write it back to the
store file.
- mobCell = mobStore.resolve(c, cacheMobBlocksOnCompaction,
false).getCell();
+ mobCell = resolveMobCell(c);
if (mobCell.getValueLength() != 0) {
// put the mob data back to the store file
PrivateCellUtil.setSequenceId(mobCell, c.getSequenceId());
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestDefaultMobStoreCompactor.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestDefaultMobStoreCompactor.java
index 510ba600cfc..945605b8211 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestDefaultMobStoreCompactor.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/mob/TestDefaultMobStoreCompactor.java
@@ -18,15 +18,21 @@
package org.apache.hadoop.hbase.mob;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.ExtendedCell;
+import org.apache.hadoop.hbase.KeyValue;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.HMobStore;
import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.hbase.util.Bytes;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
@@ -49,6 +55,27 @@ public class TestDefaultMobStoreCompactor {
assertFalse(compactor.cacheMobBlocksOnCompaction);
}
+ @Test
+ public void testResolveMobCellClosesMobCellAndReturnsIndependentCopy()
throws Exception {
+ Configuration conf = new Configuration();
+ DefaultMobStoreCompactor compactor = newCompactor(conf);
+ ExtendedCell reference = new KeyValue(Bytes.toBytes("row"),
Bytes.toBytes("family"),
+ Bytes.toBytes("qualifier"), Bytes.toBytes("mob-reference"));
+ ExtendedCell resolved = new KeyValue(Bytes.toBytes("row"),
Bytes.toBytes("family"),
+ Bytes.toBytes("qualifier"), Bytes.toBytes("mob-value"));
+ MobCell mobCell = mock(MobCell.class);
+ when(mobCell.getCell()).thenReturn(resolved);
+ when(compactor.mobStore.resolve(reference,
compactor.cacheMobBlocksOnCompaction, false))
+ .thenReturn(mobCell);
+
+ ExtendedCell copied = compactor.resolveMobCell(reference);
+
+ assertNotSame(resolved, copied);
+ assertTrue(CellUtil.matchingValue(resolved, copied));
+ verify(compactor.mobStore).resolve(reference,
compactor.cacheMobBlocksOnCompaction, false);
+ verify(mobCell).close();
+ }
+
private DefaultMobStoreCompactor newCompactor(Configuration conf) {
HMobStore store = mock(HMobStore.class);
ColumnFamilyDescriptor family = mock(ColumnFamilyDescriptor.class);
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHMobStore.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHMobStore.java
index 861d05edbb3..07bd50344f7 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHMobStore.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHMobStore.java
@@ -63,6 +63,7 @@ import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.io.crypto.MockAesKeyProvider;
import org.apache.hadoop.hbase.io.crypto.aes.AES;
import org.apache.hadoop.hbase.io.hfile.HFile;
+import org.apache.hadoop.hbase.mob.MobCell;
import org.apache.hadoop.hbase.mob.MobConstants;
import org.apache.hadoop.hbase.mob.MobFileCache;
import org.apache.hadoop.hbase.mob.MobUtils;
@@ -435,13 +436,17 @@ public class TestHMobStore {
Path targetPath = new Path(store.getPath(), targetPathName);
store.commitFile(mobFilePath, targetPath);
// resolve
- Cell resultCell1 = store.resolve(seekKey1, false).getCell();
- Cell resultCell2 = store.resolve(seekKey2, false).getCell();
- Cell resultCell3 = store.resolve(seekKey3, false).getCell();
- // compare
- assertEquals(Bytes.toString(value),
Bytes.toString(CellUtil.cloneValue(resultCell1)));
- assertEquals(Bytes.toString(value),
Bytes.toString(CellUtil.cloneValue(resultCell2)));
- assertEquals(Bytes.toString(value2),
Bytes.toString(CellUtil.cloneValue(resultCell3)));
+ try (MobCell resultCell1 = store.resolve(seekKey1, false);
+ MobCell resultCell2 = store.resolve(seekKey2, false);
+ MobCell resultCell3 = store.resolve(seekKey3, false)) {
+ // compare
+ assertEquals(Bytes.toString(value),
+ Bytes.toString(CellUtil.cloneValue(resultCell1.getCell())));
+ assertEquals(Bytes.toString(value),
+ Bytes.toString(CellUtil.cloneValue(resultCell2.getCell())));
+ assertEquals(Bytes.toString(value2),
+ Bytes.toString(CellUtil.cloneValue(resultCell3.getCell())));
+ }
}
@Test
