[GitHub] [phoenix] jpisaac commented on a diff in pull request #1569: PHOENIX-6888 Fixing TTL and Max Lookback Issues for Phoenix Tables

["jpisaac (via GitHub)" <gi...@apache.org>]

jpisaac commented on code in PR #1569:
URL: https://github.com/apache/phoenix/pull/1569#discussion_r1124839702


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java:
##########
@@ -34,14 +34,7 @@
 import org.apache.hadoop.hbase.coprocessor.RegionObserver;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.hbase.io.TimeRange;
-import org.apache.hadoop.hbase.regionserver.FlushLifeCycleTracker;
-import org.apache.hadoop.hbase.regionserver.Region;
-import org.apache.hadoop.hbase.regionserver.RegionScanner;
-import org.apache.hadoop.hbase.regionserver.ScanOptions;
-import org.apache.hadoop.hbase.regionserver.ScanType;
-import org.apache.hadoop.hbase.regionserver.ScannerContext;
-import org.apache.hadoop.hbase.regionserver.ScannerContextUtil;
-import org.apache.hadoop.hbase.regionserver.Store;
+import org.apache.hadoop.hbase.regionserver.*;

Review Comment:
   nit: remove import *



##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java:
##########
@@ -426,58 +421,39 @@ RegionScanner getWrappedScanner(final ObserverContext<RegionCoprocessorEnvironme
     public void preCompactScannerOpen(ObserverContext<RegionCoprocessorEnvironment> c, Store store,
                                              ScanType scanType, ScanOptions options, CompactionLifeCycleTracker tracker,
                                              CompactionRequest request) throws IOException {
-        Configuration conf = c.getEnvironment().getConfiguration();
-        if (isMaxLookbackTimeEnabled(conf)) {
-            setScanOptionsForFlushesAndCompactions(conf, options, store, scanType);
-        }
+        setScanOptionsForFlushesAndCompactions(options);
     }
 
     @Override
     public void preFlushScannerOpen(ObserverContext<RegionCoprocessorEnvironment> c, Store store,
                                                 ScanOptions options, FlushLifeCycleTracker tracker) throws IOException {
-        Configuration conf = c.getEnvironment().getConfiguration();
-        if (isMaxLookbackTimeEnabled(conf)) {
-            setScanOptionsForFlushesAndCompactions(conf, options, store, ScanType.COMPACT_RETAIN_DELETES);
-        }
+        setScanOptionsForFlushesAndCompactions(options);

Review Comment:
   Should the preFlush also have a specialized scanner like the StoreCompactionScanner (or maybe can be reused)?.
   As I understand the preFlush currently will keep all rows, versions and delete markers.



-- 
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-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org