[GitHub] [phoenix] gjacoby126 commented on a diff in pull request #1425: PHOENIX-6693 Remove Hbase 2.1 and Hbase 2.2 support from Phoenix

[GitBox <gi...@apache.org>]

gjacoby126 commented on code in PR #1425:
URL: https://github.com/apache/phoenix/pull/1425#discussion_r861018639


##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java:
##########
@@ -403,14 +419,165 @@ RegionScanner getWrappedScanner(final ObserverContext<RegionCoprocessorEnvironme
    /* We want to override the store scanner so that we can read "past" a delete
     marker on an SCN / lookback query to see the underlying edit. This was possible
     in HBase 1.x, but not possible after the interface changes in HBase 2.0. HBASE-24321 in
-     HBase 2.3 gave us this ability back, but we need to use it through a compatibility shim
-     so we can compile against 2.1 and 2.2. When 2.3 is the minimum supported HBase
-     version, the shim can be retired and the logic moved into the real coproc.
-
+     HBase 2.3 gave us this ability back.
     We also need to override the flush compaction coproc hooks in order to implement max lookback
      age to keep versions from being purged.
+   */
+
+    @Override
+    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);
+        }
+    }
+
+    @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);
+        }
+    }
+
+    @Override
+    public void preMemStoreCompactionCompactScannerOpen(
+        ObserverContext<RegionCoprocessorEnvironment> c, Store store, ScanOptions options)
+        throws IOException {
+        Configuration conf = c.getEnvironment().getConfiguration();
+        if (isMaxLookbackTimeEnabled(conf)) {
+            MemoryCompactionPolicy inMemPolicy =
+                store.getColumnFamilyDescriptor().getInMemoryCompaction();
+            ScanType scanType;
+            //the eager and adaptive in-memory compaction policies can purge versions; the others
+            // can't. (Eager always does; adaptive sometimes does)
+            if (inMemPolicy.equals(MemoryCompactionPolicy.EAGER) ||
+                inMemPolicy.equals(MemoryCompactionPolicy.ADAPTIVE)) {
+                scanType = ScanType.COMPACT_DROP_DELETES;
+            } else {
+                scanType = ScanType.COMPACT_RETAIN_DELETES;
+            }
+            setScanOptionsForFlushesAndCompactions(conf, options, store, scanType);
+        }
+    }
+
+    @Override
+    public void preStoreScannerOpen(ObserverContext<RegionCoprocessorEnvironment> ctx, Store store,
+                                           ScanOptions options) throws IOException {
+
+        if (!storeFileScanDoesntNeedAlteration(options)) {
+            //PHOENIX-4277 -- When doing a point-in-time (SCN) Scan, HBase by default will hide
+            // mutations that happen before a delete marker. This overrides that behavior.
+            options.setMinVersions(options.getMinVersions());
+            KeepDeletedCells keepDeletedCells = KeepDeletedCells.TRUE;
+            if (store.getColumnFamilyDescriptor().getTimeToLive() != HConstants.FOREVER) {
+                keepDeletedCells = KeepDeletedCells.TTL;
+            }
+            options.setKeepDeletedCells(keepDeletedCells);
+        }
+    }
+
+    private boolean storeFileScanDoesntNeedAlteration(ScanOptions options) {
+        Scan scan = options.getScan();
+        boolean isRaw = scan.isRaw();
+        //true if keep deleted cells is either TRUE or TTL
+        boolean keepDeletedCells = options.getKeepDeletedCells().equals(KeepDeletedCells.TRUE) ||
+            options.getKeepDeletedCells().equals(KeepDeletedCells.TTL);
+        boolean timeRangeIsLatest = scan.getTimeRange().getMax() == HConstants.LATEST_TIMESTAMP;
+        boolean timestampIsTransactional =
+            isTransactionalTimestamp(scan.getTimeRange().getMax());
+        return isRaw
+            || keepDeletedCells
+            || timeRangeIsLatest
+            || timestampIsTransactional;
+    }
+
+    private boolean isTransactionalTimestamp(long ts) {

Review Comment:
   @stoty - since we've deprecated Tephra this isTransactionalTimestamp method can probably go away. It's a hack to detect when Tephra is adjusting the HBase timestamps to be nanos, but iirc Omid doesn't change timestamps in the same way. 



##########
phoenix-core/pom.xml:
##########
@@ -545,6 +541,10 @@
       <groupId>org.apache.zookeeper</groupId>
       <artifactId>zookeeper</artifactId>
     </dependency>
+    <dependency>

Review Comment:
   This seems to be added twice, one under hadoop test dependencies and once under other dependencies



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