junegunn commented on code in PR #8628:
URL: https://github.com/apache/hbase/pull/8628#discussion_r3959870351
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/client/TableSnapshotScanner.java:
##########
@@ -160,6 +161,12 @@ private boolean isValidRegion(RegionInfo hri) {
if (hri.isOffline() && (hri.isSplit() || hri.isSplitParent())) {
return false;
}
+ // The mob region is a dummy region used only to organise mob files under
mobdir. It has no
+ // region directory under the table dir to open, and holds no rows. See
HBASE-30365 and
+ // HBASE-30368.
Review Comment:
Thank you. The fix looks right to me.
I verified it locally: the test fails without the `isValidRegion` change,
and a full scan of a restored mob snapshot still returns the real values, so
skipping the pseudo-region does not lose data.
I have one request though. I realized the last clause of the comment ("holds
no rows") is not accurate.
`DefaultMobStoreFlusher` appends the original cell to the mob file, then
writes a reference cell to the normal store:
```java
mobFileWriter.append(c);
...
// The key is same, the value is the filename of the mob file
ExtendedCell reference = MobUtils.createMobRefCell(c, fileName, ...);
writer.append(reference);
```
So the mob files do hold every row, with the real values. They just must not
be scanned as a region, since they are reached by reference from the real
regions. Simplest fix is to drop that clause:
```suggestion
// The mob region is a dummy region used only to organise mob files
under mobdir. It has no
// region directory under the table dir to open. See HBASE-30365 and
HBASE-30368.
```
That wording is mine from HBASE-30365, so the same clause is already on
master in `TableSnapshotInputFormatImpl`. Could you fix that one in this PR too?
--
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]
