bharathv commented on a change in pull request #1079:
URL: https://github.com/apache/phoenix/pull/1079#discussion_r560490725
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/iterate/TableSnapshotResultIterator.java
##########
@@ -88,19 +90,34 @@ public TableSnapshotResultIterator(Configuration
configuration, Scan scan, ScanM
}
private void init() throws IOException {
- RestoreSnapshotHelper.RestoreMetaChanges meta =
- RestoreSnapshotHelper.copySnapshotForScanner(this.configuration,
this.fs,
- this.rootDir, this.restoreDir, this.snapshotName);
- List<RegionInfo> restoredRegions = meta.getRegionsToAdd();
- this.htd = meta.getTableDescriptor();
- this.regions = new ArrayList<RegionInfo>(restoredRegions.size());
-
- for (RegionInfo restoredRegion : restoredRegions) {
- if (isValidRegion(restoredRegion)) {
- this.regions.add(restoredRegion);
+ if
(PhoenixConfigurationUtil.getMRSnapshotManagedInternally(configuration)) {
+ RestoreSnapshotHelper.RestoreMetaChanges meta =
+ RestoreSnapshotHelper.copySnapshotForScanner(this.configuration,
this.fs, this.rootDir,
+ this.restoreDir, this.snapshotName);
+ List<RegionInfo> restoredRegions = meta.getRegionsToAdd();
+ this.htd = meta.getTableDescriptor();
+ this.regions = new ArrayList<>(restoredRegions.size());
+ for (RegionInfo restoredRegion : restoredRegions) {
+ if (isValidRegion(restoredRegion)) {
+ this.regions.add(restoredRegion);
+ }
+ }
+ } else {
+ Path snapshotDir =
SnapshotDescriptionUtils.getCompletedSnapshotDir(snapshotName, rootDir);
Review comment:
Is the idea that the creator of this MR job restores this snapshot? If
yes, I think that'd be inconvenient. How about we do it in the setup and let
the InputFormat handle it? All user has to do is pass the snapshot name. Is
there some issue with this approach?
##########
File path:
phoenix-core/src/main/java/org/apache/phoenix/iterate/TableSnapshotResultIterator.java
##########
@@ -88,19 +90,34 @@ public TableSnapshotResultIterator(Configuration
configuration, Scan scan, ScanM
}
private void init() throws IOException {
- RestoreSnapshotHelper.RestoreMetaChanges meta =
- RestoreSnapshotHelper.copySnapshotForScanner(this.configuration,
this.fs,
- this.rootDir, this.restoreDir, this.snapshotName);
- List<RegionInfo> restoredRegions = meta.getRegionsToAdd();
- this.htd = meta.getTableDescriptor();
- this.regions = new ArrayList<RegionInfo>(restoredRegions.size());
-
- for (RegionInfo restoredRegion : restoredRegions) {
- if (isValidRegion(restoredRegion)) {
- this.regions.add(restoredRegion);
+ if
(PhoenixConfigurationUtil.getMRSnapshotManagedInternally(configuration)) {
Review comment:
Not sure I get this, We are again restoring per iterator which is per
scan? Isn't this what caused the issue in the first place. I see that it is
guarded by a configuration option but ideally we want to avoid this in all
cases right? or to rephrase my question, who wants to set this to true?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
