kadirozde commented on code in PR #1855:
URL: https://github.com/apache/phoenix/pull/1855#discussion_r1523942970


##########
phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java:
##########
@@ -19,6 +19,7 @@
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 
+import org.apache.hadoop.conf.Configuration;

Review Comment:
   Are these new imports used?



##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/CompactionScanner.java:
##########
@@ -84,45 +110,99 @@ public CompactionScanner(RegionCoprocessorEnvironment env,
             Store store,
             InternalScanner storeScanner,
             long maxLookbackInMillis,
-            byte[] emptyCF,
-            byte[] emptyCQ,
-            int phoenixTTL,
-            boolean isSystemTable) {
+            PTable table) throws IOException {
         this.storeScanner = storeScanner;
         this.region = env.getRegion();
         this.store = store;
         this.env = env;
-        this.emptyCF = emptyCF;
-        this.emptyCQ = emptyCQ;
+        this.emptyCF = SchemaUtil.getEmptyColumnFamily(table);
+        this.emptyCQ = table.getEncodingScheme() == 
PTable.QualifierEncodingScheme.NON_ENCODED_QUALIFIERS ?
+                QueryConstants.EMPTY_COLUMN_BYTES : 
table.getEncodingScheme().encode(QueryConstants.ENCODED_EMPTY_COLUMN_NAME);
         this.config = env.getConfiguration();
         compactionTime = EnvironmentEdgeManager.currentTimeMillis();
-        this.maxLookbackInMillis = maxLookbackInMillis;
         String columnFamilyName = store.getColumnFamilyName();
         storeColumnFamily = columnFamilyName.getBytes();
         String tableName = region.getRegionInfo().getTable().getNameAsString();
         Long overriddenMaxLookback =
                 maxLookbackMap.remove(tableName + SEPARATOR + 
columnFamilyName);
         maxLookbackInMillis = overriddenMaxLookback == null ?
                 maxLookbackInMillis : Math.max(maxLookbackInMillis, 
overriddenMaxLookback);
+
+        this.maxLookbackInMillis = maxLookbackInMillis;
         // The oldest scn is current time - maxLookbackInMillis. Phoenix sets 
the scan time range
         // for scn queries [0, scn). This means that the maxlookback size 
should be
         // maxLookbackInMillis + 1 so that the oldest scn does not return 
empty row
-        this.maxLookbackWindowStart = maxLookbackInMillis == 0 ?
-                compactionTime : compactionTime - (maxLookbackInMillis + 1);
+        this.maxLookbackWindowStart = maxLookbackInMillis == 0 ? 
compactionTime : compactionTime - (maxLookbackInMillis + 1);
         ColumnFamilyDescriptor cfd = store.getColumnFamilyDescriptor();
-        this.ttl = isSystemTable ? cfd.getTimeToLive() : phoenixTTL;
-        this.ttlWindowStart = ttl == HConstants.FOREVER ? 1 : compactionTime - 
ttl * 1000;
-        ttl *= 1000;
-        this.maxLookbackWindowStart = Math.max(ttlWindowStart, 
maxLookbackWindowStart);
         this.minVersion = cfd.getMinVersions();
         this.maxVersion = cfd.getMaxVersions();
         this.keepDeletedCells = cfd.getKeepDeletedCells();
         familyCount = region.getTableDescriptor().getColumnFamilies().length;
         localIndex = 
columnFamilyName.startsWith(LOCAL_INDEX_COLUMN_FAMILY_PREFIX);
-        emptyCFStore = familyCount == 1 || 
columnFamilyName.equals(Bytes.toString(emptyCF))
-                        || localIndex;
-        phoenixLevelRowCompactor = new PhoenixLevelRowCompactor();
-        hBaseLevelRowCompactor = new HBaseLevelRowCompactor();
+        emptyCFStore = familyCount == 1 || 
columnFamilyName.equals(Bytes.toString(emptyCF)) || localIndex;
+        // TODO: check if is it appropriate to throw an IOException here

Review Comment:
   It should be okay to generate an exception here. What is the reason for 
throwing an exception?



##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/CompactionScanner.java:
##########
@@ -63,8 +89,8 @@ public class CompactionScanner implements InternalScanner {
     private final Configuration config;
     private final RegionCoprocessorEnvironment env;
     private long maxLookbackWindowStart;
-    private long ttlWindowStart;
-    private long ttl;
+    //private long ttlWindowStart;

Review Comment:
   These commented lines should be removed



##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/CompactionScanner.java:
##########
@@ -160,17 +240,390 @@ public void close() throws IOException {
         storeScanner.close();
     }
 
+    private enum MatcherType {
+        VIEW_INDEXES, GLOBAL_VIEWS, TENANT_VIEWS
+    }
+
+    private interface TTLTracker {
+        void setTTL(Cell firstCell);
+        RowContext getRowContext();
+    }
+    private class NonPartitionedTableTTLTracker implements TTLTracker {
+
+        private long ttl;
+        private RowContext rowContext;
+
+        public NonPartitionedTableTTLTracker(PTable pTable, 
RegionCoprocessorEnvironment env, Store store) {
+            boolean isSystemTable = pTable.getType() == PTableType.SYSTEM;
+            if (isSystemTable) {
+                ColumnFamilyDescriptor cfd = store.getColumnFamilyDescriptor();
+                ttl = cfd.getTimeToLive();
+            } else {
+                ttl = pTable.getTTL() != TTL_NOT_DEFINED ? pTable.getTTL() : 
DEFAULT_TTL;
+            }
+            LOGGER.info(String.format("NonPartitionedTableTTLTracker params:- 
(physical-name=%s, ttl=%d, isSystemTable=%s)",
+                    pTable.getName().toString(), ttl*1000, isSystemTable));
+        }
+
+        @Override
+        public void setTTL(Cell firstCell) {
+            this.rowContext = new RowContext();
+            this.rowContext.setTTL(ttl);
+
+        }
+
+        @Override
+        public RowContext getRowContext() {
+            if (this.rowContext == null) {
+                this.rowContext = new RowContext();
+                this.rowContext.setTTL(ttl);
+            }
+            return rowContext;
+        }
+    }
+
+    private class PartitionedTableTTLTracker implements TTLTracker {
+        private final Logger LOGGER = LoggerFactory.getLogger(
+                PartitionedTableTTLTracker.class);
+        private PTable baseTable;
+        private TableTTLInfoCache ttlCache;
+        private RowKeyMatcher globalViewMatcher;
+        private RowKeyMatcher tenantViewMatcher;
+        private RowKeyMatcher viewIndexMatcher;
+
+        // Default or Table-Level TTL
+        private long ttl;
+        private RowContext rowContext;
+
+        private boolean isIndexTable = false;
+        private boolean isMultiTenant = false;
+        private boolean isSalted = false;
+        private int startingPKPosition;
+        private RowKeyParser rowKeyParser;
+
+        public PartitionedTableTTLTracker(PTable table, 
RegionCoprocessorEnvironment env,
+                boolean isSalted, boolean isIndexTable) {
+
+            try {
+                this.baseTable = table;
+                this.ttlCache = new TableTTLInfoCache();
+                this.rowKeyParser = new RowKeyParser(baseTable);
+                this.ttl = table.getTTL() != TTL_NOT_DEFINED ? table.getTTL() 
: DEFAULT_TTL;
+                this.isIndexTable = isIndexTable || localIndex ;
+                this.isSalted = isSalted;
+                this.isMultiTenant = table.isMultiTenant();
+
+                int startingPKPosition = 0;
+                if (this.isMultiTenant && this.isSalted && this.isIndexTable) {

Review Comment:
   Can we turn this into more readable if then else statements by checking only 
one condition per if statement? I think it is better to create a lookup table 
for this, for example, int[][][] StartingPKPosition = new int[2][2][2];



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to