HBASE-27826 Refactor code to move creation of Ref files to SFT interface apis #5834
+627
−238
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -666,8 +666,9 @@ private Pair<List<Path>, List<Path>> splitStoreFiles(final MasterProcedureEnv en | |
| // table dir. In case of failure, the proc would go through this again, already existing | ||
| // region dirs and split files would just be ignored, new split files should get created. | ||
| int nbFiles = 0; | ||
| final Map<String, Pair<Collection<StoreFileInfo>, StoreFileTracker>> files = | ||
| new HashMap<String, Pair<Collection<StoreFileInfo>, StoreFileTracker>>( | ||
| htd.getColumnFamilyCount()); | ||
| for (ColumnFamilyDescriptor cfd : htd.getColumnFamilies()) { | ||
| String family = cfd.getNameAsString(); | ||
| StoreFileTracker tracker = | ||
|
|
@@ -690,7 +691,7 @@ private Pair<List<Path>, List<Path>> splitStoreFiles(final MasterProcedureEnv en | |
| } | ||
| if (filteredSfis == null) { | ||
| filteredSfis = new ArrayList<StoreFileInfo>(sfis.size()); | ||
| files.put(family, new Pair(filteredSfis, tracker)); | ||
| } | ||
| filteredSfis.add(sfi); | ||
| nbFiles++; | ||
|
|
@@ -713,10 +714,11 @@ private Pair<List<Path>, List<Path>> splitStoreFiles(final MasterProcedureEnv en | |
| final List<Future<Pair<Path, Path>>> futures = new ArrayList<Future<Pair<Path, Path>>>(nbFiles); | ||
|
|
||
| // Split each store file. | ||
| for (Map.Entry<String, Pair<Collection<StoreFileInfo>, StoreFileTracker>> e : files | ||
| .entrySet()) { | ||
| byte[] familyName = Bytes.toBytes(e.getKey()); | ||
| final ColumnFamilyDescriptor hcd = htd.getColumnFamily(familyName); | ||
| final Collection<StoreFileInfo> storeFiles = e.getValue().getFirst(); | ||
| if (storeFiles != null && storeFiles.size() > 0) { | ||
| final Configuration storeConfiguration = | ||
| StoreUtils.createStoreConfiguration(env.getMasterConfiguration(), htd, hcd); | ||
|
|
@@ -727,8 +729,9 @@ private Pair<List<Path>, List<Path>> splitStoreFiles(final MasterProcedureEnv en | |
| // is running in a regionserver's Store context, or we might not be able | ||
| // to read the hfiles. | ||
| storeFileInfo.setConf(storeConfiguration); | ||
| StoreFileSplitter sfs = | ||
| new StoreFileSplitter(regionFs, e.getValue().getSecond(), familyName, | ||
| new HStoreFile(storeFileInfo, hcd.getBloomFilterType(), CacheConfig.DISABLED)); | ||
| futures.add(threadPool.submit(sfs)); | ||
| } | ||
| } | ||
|
|
@@ -794,19 +797,19 @@ private void assertSplitResultFilesCount(final FileSystem fs, | |
| } | ||
| } | ||
|
|
||
| private Pair<Path, Path> splitStoreFile(HRegionFileSystem regionFs, StoreFileTracker tracker, | ||
| byte[] family, HStoreFile sf) throws IOException { | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("pid=" + getProcId() + " splitting started for store file: " + sf.getPath() | ||
| + " for region: " + getParentRegion().getShortNameToLog()); | ||
| } | ||
|
|
||
| final byte[] splitRow = getSplitRow(); | ||
| final String familyName = Bytes.toString(family); | ||
| final Path path_first = regionFs.splitStoreFile(this.daughterOneRI, familyName, sf, splitRow, | ||
| false, splitPolicy, tracker); | ||
| final Path path_second = regionFs.splitStoreFile(this.daughterTwoRI, familyName, sf, splitRow, | ||
| true, splitPolicy, tracker); | ||
| if (LOG.isDebugEnabled()) { | ||
| LOG.debug("pid=" + getProcId() + " splitting complete for store file: " + sf.getPath() | ||
| + " for region: " + getParentRegion().getShortNameToLog()); | ||
|
|
@@ -822,22 +825,25 @@ private class StoreFileSplitter implements Callable<Pair<Path, Path>> { | |
| private final HRegionFileSystem regionFs; | ||
| private final byte[] family; | ||
| private final HStoreFile sf; | ||
| private final StoreFileTracker tracker; | ||
|
|
||
| /** | ||
| * Constructor that takes what it needs to split | ||
| * @param regionFs the file system | ||
| * @param family Family that contains the store file | ||
| * @param sf which file | ||
| */ | ||
| public StoreFileSplitter(HRegionFileSystem regionFs, StoreFileTracker tracker, byte[] family, | ||
| HStoreFile sf) { | ||
| this.regionFs = regionFs; | ||
| this.sf = sf; | ||
| this.family = family; | ||
| this.tracker = tracker; | ||
| } | ||
|
|
||
| @Override | ||
| public Pair<Path, Path> call() throws IOException { | ||
| return splitStoreFile(regionFs, tracker, family, sf); | ||
|
Contributor
There was a problem hiding this comment. I think here we need to abstract at a higer level. As if we use file based store file tracker, we do not need multi threading. So we'd better abstract a method for splitting multiple store files in the store file tracker interface, and in the implementation, we are free to choose whether to use multi threading.
Contributor
Author
There was a problem hiding this comment. Yes if we can add this api as well in SFT, it will be easier to choose if where we want multi threading, but both impls might still need multi threading while initiating reader for all the parent store files while reading metadata for first/lastkeys. |
||
| } | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a separate StoreFileTracker for every StoreFile? Seems strange...
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No we have a StoreFileTracker per column family here, we refactored current struct which contains [columnFamilyName] ---> [ Collection(StoreFileInfo) ] to
[columnFamilyName] ---> [ Collection(StoreFileInfo) + sft ]