[GitHub] incubator-tephra pull request #61: TEPHRA-263 Enforce TTL, regardless of any...

[anew]

Github user anew commented on a diff in the pull request:

https://github.com/apache/incubator-tephra/pull/61#discussion_r139287848
  
--- Diff: 
tephra-hbase-compat-1.0/src/test/java/org/apache/tephra/hbase/coprocessor/TransactionVisibilityFilterTest.java
 ---
@@ -290,44 +290,52 @@ private void runFilteringTest(TxFilterFactory 
txFilterFactory,
   @Test
   public void testTTLFiltering() throws Exception {
 Map ttls = Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
-ttls.put(FAM, 10L);
-ttls.put(FAM2, 30L);
+ttls.put(FAM, 100L);
+ttls.put(FAM2, 300L);
 ttls.put(FAM3, 0L);
 
+// start a transaction to populate the in-progress list. It should not 
affect TTL calculations
+txManager.startShort();
--- End diff --

abort this tx at the end of the test?


---

[GitHub] incubator-tephra pull request #61: TEPHRA-263 Enforce TTL, regardless of any...

[anew]

Github user anew commented on a diff in the pull request:

https://github.com/apache/incubator-tephra/pull/61#discussion_r139287786
  
--- Diff: tephra-core/src/main/java/org/apache/tephra/util/TxUtils.java ---
@@ -75,12 +75,9 @@ public static long getOldestVisibleTimestamp(Map ttlByFamily, Tran
* @return The oldest timestamp that will be visible for the given 
transaction and TTL configuration
*/
   public static long getOldestVisibleTimestamp(Map 
ttlByFamily, Transaction tx, boolean readNonTxnData) {
-if (readNonTxnData) {
-  long maxTTL = getMaxTTL(ttlByFamily);
-  return maxTTL < Long.MAX_VALUE ? System.currentTimeMillis() - maxTTL 
: 0;
-}
-
-return getOldestVisibleTimestamp(ttlByFamily, tx);
+long ttlFactor = readNonTxnData ? 1 : TxConstants.MAX_TX_PER_MS;
+long maxTTL = getMaxTTL(ttlByFamily);
+return maxTTL < Long.MAX_VALUE ? tx.getTransactionId() - maxTTL * 
ttlFactor : 0;
--- End diff --

this can be negative if a large TTL is used. 


---

[GitHub] incubator-tephra pull request #61: TEPHRA-263 Enforce TTL, regardless of any...

[anew]

Github user anew commented on a diff in the pull request:

https://github.com/apache/incubator-tephra/pull/61#discussion_r139287927
  
--- Diff: 
tephra-hbase-compat-1.0/src/test/java/org/apache/tephra/hbase/coprocessor/TransactionVisibilityFilterTest.java
 ---
@@ -290,44 +290,52 @@ private void runFilteringTest(TxFilterFactory 
txFilterFactory,
   @Test
   public void testTTLFiltering() throws Exception {
 Map ttls = Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
-ttls.put(FAM, 10L);
-ttls.put(FAM2, 30L);
+ttls.put(FAM, 100L);
+ttls.put(FAM2, 300L);
 ttls.put(FAM3, 0L);
 
+// start a transaction to populate the in-progress list. It should not 
affect TTL calculations
+txManager.startShort();
+
+// commit a transaction, just to move the readPointer. Otherwise, the 
TransactionVisibilityFilter will filter
+// based upon the readPointer being smaller than the tested values, 
whereas we want to test the TTL filtering
+Transaction superShortTx = txManager.startShort();
--- End diff --

actually, you don't really need the txManager for this test. You can simply 
construct a transaction that has the desired properties. 


---

[jira] [Commented] (TEPHRA-263) TTL is not strictly enforced, if there are long transactions running

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TEPHRA-263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16168934#comment-16168934
 ] 

ASF GitHub Bot commented on TEPHRA-263:
---

Github user anew commented on a diff in the pull request:

https://github.com/apache/incubator-tephra/pull/61#discussion_r139287786
  
--- Diff: tephra-core/src/main/java/org/apache/tephra/util/TxUtils.java ---
@@ -75,12 +75,9 @@ public static long getOldestVisibleTimestamp(Map ttlByFamily, Tran
* @return The oldest timestamp that will be visible for the given 
transaction and TTL configuration
*/
   public static long getOldestVisibleTimestamp(Map 
ttlByFamily, Transaction tx, boolean readNonTxnData) {
-if (readNonTxnData) {
-  long maxTTL = getMaxTTL(ttlByFamily);
-  return maxTTL < Long.MAX_VALUE ? System.currentTimeMillis() - maxTTL 
: 0;
-}
-
-return getOldestVisibleTimestamp(ttlByFamily, tx);
+long ttlFactor = readNonTxnData ? 1 : TxConstants.MAX_TX_PER_MS;
+long maxTTL = getMaxTTL(ttlByFamily);
+return maxTTL < Long.MAX_VALUE ? tx.getTransactionId() - maxTTL * 
ttlFactor : 0;
--- End diff --

this can be negative if a large TTL is used. 


> TTL is not strictly enforced, if there are long transactions running
> 
>
> Key: TEPHRA-263
> URL: https://issues.apache.org/jira/browse/TEPHRA-263
> Project: Tephra
>  Issue Type: Bug
>Reporter: Ali Anwar
>Assignee: Ali Anwar
>
> The logic for filtering for TTL:
> https://github.com/apache/incubator-tephra/blob/release/0.12.0-incubating/tephra-core/src/main/java/org/apache/tephra/util/TxUtils.java#L66
> It is subtracting the TTL duration from the visibility upper bound, but it 
> should be subtracting from the current time or the current write pointer 
> instead. Otherwise, if the TTL is 1 hour, but the visibility upper bound is 
> 22 hours ago (due to some MR that has been in progress for 22 hours), then 
> the TTL that is actually enforced will be 23 hours and older data will be 
> filtered.
> After the long transactions are invalidated, the TTL is then strictly 
> enforced.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (TEPHRA-263) TTL is not strictly enforced, if there are long transactions running

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TEPHRA-263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16168933#comment-16168933
 ] 

ASF GitHub Bot commented on TEPHRA-263:
---

Github user anew commented on a diff in the pull request:

https://github.com/apache/incubator-tephra/pull/61#discussion_r139287927
  
--- Diff: 
tephra-hbase-compat-1.0/src/test/java/org/apache/tephra/hbase/coprocessor/TransactionVisibilityFilterTest.java
 ---
@@ -290,44 +290,52 @@ private void runFilteringTest(TxFilterFactory 
txFilterFactory,
   @Test
   public void testTTLFiltering() throws Exception {
 Map ttls = Maps.newTreeMap(Bytes.BYTES_COMPARATOR);
-ttls.put(FAM, 10L);
-ttls.put(FAM2, 30L);
+ttls.put(FAM, 100L);
+ttls.put(FAM2, 300L);
 ttls.put(FAM3, 0L);
 
+// start a transaction to populate the in-progress list. It should not 
affect TTL calculations
+txManager.startShort();
+
+// commit a transaction, just to move the readPointer. Otherwise, the 
TransactionVisibilityFilter will filter
+// based upon the readPointer being smaller than the tested values, 
whereas we want to test the TTL filtering
+Transaction superShortTx = txManager.startShort();
--- End diff --

actually, you don't really need the txManager for this test. You can simply 
construct a transaction that has the desired properties. 


> TTL is not strictly enforced, if there are long transactions running
> 
>
> Key: TEPHRA-263
> URL: https://issues.apache.org/jira/browse/TEPHRA-263
> Project: Tephra
>  Issue Type: Bug
>Reporter: Ali Anwar
>Assignee: Ali Anwar
>
> The logic for filtering for TTL:
> https://github.com/apache/incubator-tephra/blob/release/0.12.0-incubating/tephra-core/src/main/java/org/apache/tephra/util/TxUtils.java#L66
> It is subtracting the TTL duration from the visibility upper bound, but it 
> should be subtracting from the current time or the current write pointer 
> instead. Otherwise, if the TTL is 1 hour, but the visibility upper bound is 
> 22 hours ago (due to some MR that has been in progress for 22 hours), then 
> the TTL that is actually enforced will be 23 hours and older data will be 
> filtered.
> After the long transactions are invalidated, the TTL is then strictly 
> enforced.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)