gvprathyusha6 commented on code in PR #5939:
URL: https://github.com/apache/hbase/pull/5939#discussion_r1773578872
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/CatalogJanitor.java:
##########
@@ -422,7 +426,16 @@ private static Pair<Boolean, Boolean>
checkRegionReferences(MasterServices servi
try {
HRegionFileSystem regionFs = HRegionFileSystem
.openRegionFromFileSystem(services.getConfiguration(), fs, tabledir,
region, true);
- boolean references = regionFs.hasReferences(tableDescriptor);
+ ColumnFamilyDescriptor[] families = tableDescriptor.getColumnFamilies();
+ boolean references = false;
+ for (ColumnFamilyDescriptor cfd : families) {
+ StoreFileTracker sft =
StoreFileTrackerFactory.create(services.getConfiguration(),
+ tableDescriptor,
ColumnFamilyDescriptorBuilder.of(cfd.getNameAsString()), regionFs);
+ references = references || sft.hasReferences();
+ if (references) {
+ break;
+ }
+ }
Review Comment:
> Should also be moved to HRegionFileSystem.hasReferences(), like other
methods did?
When we introduce virtual links to Link/Ref files using SFT,
hasReferences() abstraction also should move to SFT layer and there is only one
reference left of HRegionFileSystem.hasReferences() which will be eventually
removed in [HBASE-28861](https://issues.apache.org/jira/browse/HBASE-28861)
> Is it guaranteed that we don't have more than one SFT instance for the
same store at any point in time? Because the file based SFT impl relies on the
fact it's the single instance manipulating its meta files
This should only be true for WRITE mode of SFT instance right? there could
be multiple instances of READ only mode SFT for the same store. I have created
HBASE-28863 for introducing cache primarily for READ only mode
>If we have multiple instances of SFT for the same store, it could lead to
inconsistencies, where one of the instances update the meta file, than the
others would be looking at an outdated state of the meta files.
Is this part not already handled based on timestamp? Or Flush and Compaction
both are using SFT instance from StoreEngine due to that same reason?
Also looks like at couple of places I am using non READ only mode and I was
only needing READ mode there, let me quickly reiterate on that, but I guess it
should be good to have that check at StoreFileTrackerFactory layer as well.
thanks for pointing that out @wchevreuil
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
