liuxiaocs7 commented on code in PR #8392:
URL: https://github.com/apache/hbase/pull/8392#discussion_r3461707218
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java:
##########
@@ -174,6 +175,29 @@ public DefaultMobStoreCompactor(Configuration conf, HStore
store) {
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)) {
Review Comment:
Thanks @guluo2016 ! The false (readEmptyValueOnMobCellMiss) is coupled to
this path's miss handling (it relies on resolve throwing, while true returns an
empty cell needing different handling), so it can't be reused as-is. I'd prefer
to add the param with a proper name once a real true caller appears — WDYT?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java:
##########
@@ -174,6 +175,29 @@ public DefaultMobStoreCompactor(Configuration conf, HStore
store) {
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)) {
Review Comment:
Thanks @guluo2016 ! The false (readEmptyValueOnMobCellMiss) is coupled to
this path's miss handling (it relies on resolve throwing, while true returns an
empty cell needing different handling), so it can't be reused as-is. I'd prefer
to add the param with a proper name once a real true caller appears, WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
