wchevreuil commented on a change in pull request #3685:
URL: https://github.com/apache/hbase/pull/3685#discussion_r709917112
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotManifest.java
##########
@@ -317,26 +318,19 @@ protected void addRegion(final Path tableDir, final
RegionInfo regionInfo, Regio
// in batches and may miss files being added/deleted. This could be more
robust (iteratively
// checking to see if we have all the files until we are sure), but the
limit is currently
// 1000 files/batch, far more than the number of store files under a
single column family.
- Collection<String> familyNames = regionFs.getFamilies();
- if (familyNames != null) {
- for (String familyName: familyNames) {
- Object familyData = visitor.familyOpen(regionData,
Bytes.toBytes(familyName));
- monitor.rethrowException();
-
- Collection<StoreFileInfo> storeFiles =
regionFs.getStoreFiles(familyName);
- if (storeFiles == null) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("No files under family: " + familyName);
- }
- continue;
- }
-
- // 2.1. build the snapshot reference for the store
- // iterate through all the store's files and create "references".
- addReferenceFiles(visitor, regionData, familyData, storeFiles,
false);
-
- visitor.familyClose(regionData, familyData);
+ for (ColumnFamilyDescriptor cfd : htd.getColumnFamilies()) {
+ Object familyData = visitor.familyOpen(regionData, cfd.getName());
+ monitor.rethrowException();
+ StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, htd,
cfd, regionFs);
Review comment:
Similar to my above comment. We are assuming conf has the proper store
file tracker impl config, but this might be defined at table or CF level.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java
##########
@@ -612,9 +610,7 @@ private void createMergedRegion(final MasterProcedureEnv
env) throws IOException
List<Path> mergedFiles = new ArrayList<>();
for (ColumnFamilyDescriptor hcd : htd.getColumnFamilies()) {
String family = hcd.getNameAsString();
- Configuration trackerConfig =
-
StoreFileTrackerFactory.mergeConfigurations(env.getMasterConfiguration(), htd,
hcd);
- StoreFileTracker tracker = StoreFileTrackerFactory.create(trackerConfig,
family, regionFs);
+ StoreFileTracker tracker =
StoreFileTrackerFactory.create(env.getMasterConfiguration(),htd, hcd, regionFs);
Review comment:
We don't need merge master config anymore? I believe we still need this,
maybe we can move this merge logic to "StoreFileTrackerFactory.create" itself,
but currently `StoreFileTrackerFactory.create` is only checking the passed
config instance for the store file tracker impl.
--
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]
