This is an automated email from the ASF dual-hosted git repository.
liuxiaocs7 pushed a commit to branch branch-2.5
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.5 by this push:
new 9739d484e05 HBASE-30246 MOB compaction does not close MobCell after
resolving reference cells (#8422) (#8425)
9739d484e05 is described below
commit 9739d484e0501c62360b5e5ea48a95c71accf9a6
Author: Xiao Liu <[email protected]>
AuthorDate: Mon Jun 29 10:32:46 2026 +0800
HBASE-30246 MOB compaction does not close MobCell after resolving reference
cells (#8422) (#8425)
Signed-off-by: Peng Lu <[email protected]>
(cherry picked from commit 8667763eb93d9954c8d834ca6594ed5c3baafa40)
(cherry picked from commit 5ba181e17af1c70f29471315f601b6cf5ccdb271)
---
.../hadoop/hbase/mob/DefaultMobStoreCompactor.java | 25 +++++++++++++++++++-
.../hadoop/hbase/mob/FaultyMobStoreCompactor.java | 4 ++--
.../hbase/mob/TestDefaultMobStoreCompactor.java | 27 ++++++++++++++++++++++
.../hadoop/hbase/regionserver/TestHMobStore.java | 19 +++++++++------
4 files changed, 65 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 ed7264cc89c..4d8d946d14e 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
@@ -36,6 +36,7 @@ import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.Cell;
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;
@@ -173,6 +174,28 @@ 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(Cell)} 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 Cell resolveMobCell(Cell 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 {
@@ -369,7 +392,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 (FileNotFoundException fnfe) {
if (discardMobMiss) {
LOG.error("Missing MOB cell: file={} not found cell={}",
fName, c);
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 d807be32cb9..82c218352f2 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
@@ -188,7 +188,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 (FileNotFoundException fnfe) {
if (discardMobMiss) {
LOG.error("Missing MOB cell: file={} not found", fName);
@@ -255,7 +255,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..b695299777b 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.Cell;
+import org.apache.hadoop.hbase.CellUtil;
+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);
+ Cell reference = new KeyValue(Bytes.toBytes("row"),
Bytes.toBytes("family"),
+ Bytes.toBytes("qualifier"), Bytes.toBytes("mob-reference"));
+ Cell 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);
+
+ Cell 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 b09260d1b73..b80335e1c45 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
@@ -60,6 +60,7 @@ import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
import org.apache.hadoop.hbase.io.crypto.KeyProviderForTesting;
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;
@@ -431,13 +432,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())));
+ }
}
/**
