ohadshacham commented on a change in pull request #59: [OMID-146] Fix 
consistensy of compaction when the commit time stamp has…
URL: https://github.com/apache/incubator-omid/pull/59#discussion_r277984747
 
 

 ##########
 File path: 
hbase-coprocessor/src/main/java/org/apache/omid/transaction/CompactorScanner.java
 ##########
 @@ -228,43 +228,90 @@ private long getLowWatermarkFromCommitTable() throws 
IOException {
         }
     }
 
-    private Optional<CommitTimestamp> queryCommitTimestamp(Cell cell) throws 
IOException {
-        Optional<CommitTimestamp> cachedValue = 
commitCache.get(cell.getTimestamp());
-        if (cachedValue != null) {
-            return cachedValue;
-        }
+
+    private Result getShadowCell(byte[] row, byte[] family, byte[] qualifier, 
long timestamp) throws IOException {
+        Get g = new Get(row);
+        g.addColumn(family, qualifier);
+        g.setTimeStamp(timestamp);
+        Result r = hRegion.get(g);
+        return r;
+    }
+
+
+    private Optional<CommitTimestamp> getCommitTimestampWithRaces(Cell cell) 
throws IOException {
         try {
+            byte[] family = CellUtil.cloneFamily(cell);
+            byte[] qualifier = 
CellUtils.addShadowCellSuffixPrefix(cell.getQualifierArray(),
+                    cell.getQualifierOffset(),
+                    cell.getQualifierLength());
+            // 2) Then check the commit table
             Optional<CommitTimestamp> ct = 
commitTableClient.getCommitTimestamp(cell.getTimestamp()).get();
             if (ct.isPresent()) {
-                commitCache.put(cell.getTimestamp(), ct);
-                return Optional.of(ct.get());
-            } else {
-                Get g = new Get(CellUtil.cloneRow(cell));
-                byte[] family = CellUtil.cloneFamily(cell);
-                byte[] qualifier = 
CellUtils.addShadowCellSuffixPrefix(cell.getQualifierArray(),
-                        cell.getQualifierOffset(),
-                        cell.getQualifierLength());
-                g.addColumn(family, qualifier);
-                g.setTimeStamp(cell.getTimestamp());
-                Result r = hRegion.get(g);
-                if (r.containsColumn(family, qualifier)) {
+                if (ct.get().isValid()) {
+                    return Optional.of(ct.get());
+                }
+                // If invalid still should check sc because maybe we are in 
lowlatency mode.
+            }
+
+            // 3) Read from shadow cell
+            Result r = getShadowCell(CellUtil.cloneRow(cell), family, 
qualifier, cell.getTimestamp());
+            if (r.containsColumn(CellUtil.cloneFamily(cell), qualifier)) {
+                Optional<CommitTimestamp> retval = Optional.of(new 
CommitTimestamp(SHADOW_CELL,
+                        Bytes.toLong(r.getValue(family, qualifier)), true));
+                return retval;
+            }
+
+            // [OMID-146] - we have to invalidate a transaction if it hasn't 
reached the commit table
+            // 4) invalidate the entry
+            Boolean invalidated = 
commitTableClient.tryInvalidateTransaction(cell.getTimestamp()).get();
+            if (invalidated) {
+                // If we are running lowLatency Omid, we could have manged to 
invalidate a ct entry,
+                // but the committing client already wrote to shadow cells:
+                Result r2 = getShadowCell(CellUtil.cloneRow(cell), family, 
qualifier, cell.getTimestamp());
+                if (r2.containsColumn(CellUtil.cloneFamily(cell), qualifier)) {
                     Optional<CommitTimestamp> retval = Optional.of(new 
CommitTimestamp(SHADOW_CELL,
-                            Bytes.toLong(r.getValue(family, qualifier)), 
true));
-                    commitCache.put(cell.getTimestamp(), retval);
+                            Bytes.toLong(r2.getValue(family, qualifier)), 
true));
                     return retval;
-
                 }
+                return Optional.absent();
+            }
+
+            // 5) We did not manage to invalidate the transactions then check 
the commit table
+            Optional<CommitTimestamp> ct2 = 
commitTableClient.getCommitTimestamp(cell.getTimestamp()).get();
+            if (ct2.isPresent()) {
+                return Optional.of(ct2.get());
 
 Review comment:
   Shouldn't we check "if (ct.get().isValid()) {" and return absent in case it 
is not? under the "if (ct2.isPresent()) {".

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to