sakshamgangwar commented on a change in pull request #1079:
URL: https://github.com/apache/phoenix/pull/1079#discussion_r562264530



##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/iterate/TableSnapshotResultIterator.java
##########
@@ -80,7 +81,12 @@ public TableSnapshotResultIterator(Configuration 
configuration, Scan scan, ScanM
     this.scan = scan;
     this.scanMetricsHolder = scanMetricsHolder;
     this.scanIterator = UNINITIALIZED_SCANNER;
-    this.restoreDir = new 
Path(configuration.get(PhoenixConfigurationUtil.RESTORE_DIR_KEY));
+    if (PhoenixConfigurationUtil.isMRSnapshotManagedExternally(configuration)) 
{
+      this.restoreDir = new 
Path(configuration.get(PhoenixConfigurationUtil.RESTORE_DIR_KEY));
+    } else {
+      this.restoreDir = new 
Path(configuration.get(PhoenixConfigurationUtil.RESTORE_DIR_KEY),

Review comment:
       @gjacoby126 that existed before this change as well, this is not a 
necessary code, I believe at job level in PhoenixMapReduceUtil we update the 
configuration restoreDir to a new path so as to make sure that even if 
restoreDir configuration sent from the caller is same for multiple jobs, it 
should be able to handle it internally by adding a new sub-directory (which we 
are also doing at scan level too :) ). Whereas in externally managed now we 
take full responsibility for the restoreDir creation/usage/deletion.




----------------------------------------------------------------
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]


Reply via email to