You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/12/02 15:08:36 UTC

[GitHub] [hive] vcsomor opened a new pull request #2837: HIVE-25716 Fix of flaky test TestCompactionMetrics#testOldestReadyForCleaningAge

vcsomor opened a new pull request #2837:
URL: https://github.com/apache/hive/pull/2837


   Fix of flaky test TestCompactionMetrics#testOldestReadyForCleaningAge


-- 
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: gitbox-unsubscribe@hive.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] deniskuzZ commented on a change in pull request #2837: HIVE-25716 Fix of flaky test TestCompactionMetrics#testOldestReadyForCleaningAge

Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #2837:
URL: https://github.com/apache/hive/pull/2837#discussion_r761821097



##########
File path: ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestCompactionMetrics.java
##########
@@ -178,41 +178,42 @@ public void testInitiatorPerfMetricsDisabled() throws Exception {
   }
 
   @Test
-  @org.junit.Ignore("HIVE-25716")
   public void testOldestReadyForCleaningAge() throws Exception {
     conf.setIntVar(HiveConf.ConfVars.COMPACTOR_MAX_NUM_DELTA, 1);
 
-    long oldStart = System.currentTimeMillis();
-    Table old = newTable("default", "old_rfc", true);
-    Partition oldP = newPartition(old, "part");
+    final String DB_NAME = "default";
+    final String OLD_TABLE_NAME = "old_rfc";
+    final String OLD_PARTITION_NAME = "part";
+    final String YOUNG_TABLE_NAME = "young_rfc";
+    final String YOUNG_PARTITION_NAME = "part";
+
+    Table old = newTable(DB_NAME, OLD_TABLE_NAME, true);
+    Partition oldP = newPartition(old, OLD_PARTITION_NAME);
     addBaseFile(old, oldP, 20L, 20);
     addDeltaFile(old, oldP, 21L, 22L, 2);
     addDeltaFile(old, oldP, 23L, 24L, 2);
-    burnThroughTransactions("default", "old_rfc", 25);
-    CompactionRequest rqst = new CompactionRequest("default", "old_rfc", CompactionType.MINOR);
-    rqst.setPartitionname("ds=part");
-    txnHandler.compact(rqst);
-    startWorker();
+    burnThroughTransactions(DB_NAME, OLD_TABLE_NAME, 25);
+    doCompaction(DB_NAME, OLD_TABLE_NAME, OLD_PARTITION_NAME, CompactionType.MINOR);
+    long oldTableClosestCqCommitTime = System.currentTimeMillis();
 
-    long youngStart = System.currentTimeMillis();
-    Table young = newTable("default", "young_rfc", true);
-    Partition youngP = newPartition(young, "part");
+    Table young = newTable(DB_NAME, YOUNG_TABLE_NAME, true);
+    Partition youngP = newPartition(young, YOUNG_PARTITION_NAME);
     addBaseFile(young, youngP, 20L, 20);
     addDeltaFile(young, youngP, 21L, 22L, 2);
     addDeltaFile(young, youngP, 23L, 24L, 2);
-    burnThroughTransactions("default", "young_rfc", 25);
-    rqst = new CompactionRequest("default", "young_rfc", CompactionType.MINOR);
-    rqst.setPartitionname("ds=part");
-    txnHandler.compact(rqst);
-    startWorker();
+    burnThroughTransactions(DB_NAME, YOUNG_TABLE_NAME, 25);
+    doCompaction(DB_NAME, YOUNG_TABLE_NAME, YOUNG_PARTITION_NAME, CompactionType.MINOR);
+    long youngTableClosestCqCommitTime = System.currentTimeMillis();
 
     runAcidMetricService();
-    long oldDiff = (System.currentTimeMillis() - oldStart)/1000;
-    long youngDiff = (System.currentTimeMillis() - youngStart)/1000;
+    long nowClosestToAcidMetricServiceEnd = System.currentTimeMillis();
 
-    long threshold = 1000;
-    Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.OLDEST_READY_FOR_CLEANING_AGE).intValue() <= oldDiff + threshold);
-    Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.OLDEST_READY_FOR_CLEANING_AGE).intValue() >= youngDiff);
+    long oldAgeInSeconds = (nowClosestToAcidMetricServiceEnd - oldTableClosestCqCommitTime) / 1000;
+    long youngAgeInSeconds = (nowClosestToAcidMetricServiceEnd - youngTableClosestCqCommitTime) / 1000;
+
+    int gaugeValue = Metrics.getOrCreateGauge(MetricsConstants.OLDEST_READY_FOR_CLEANING_AGE).intValue();
+    Assert.assertTrue(gaugeValue <= oldAgeInSeconds);

Review comment:
       why not check that the gauge value is in some range of `oldAgeInSeconds` (+- 10ms)? Not sure why youngAgeInSeconds is even relevant here.




-- 
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: gitbox-unsubscribe@hive.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] asinkovits merged pull request #2837: HIVE-25716 Fix of flaky test TestCompactionMetrics#testOldestReadyForCleaningAge

Posted by GitBox <gi...@apache.org>.
asinkovits merged pull request #2837:
URL: https://github.com/apache/hive/pull/2837


   


-- 
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: gitbox-unsubscribe@hive.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] deniskuzZ commented on a change in pull request #2837: HIVE-25716 Fix of flaky test TestCompactionMetrics#testOldestReadyForCleaningAge

Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #2837:
URL: https://github.com/apache/hive/pull/2837#discussion_r761822390



##########
File path: ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/CompactorTest.java
##########
@@ -292,22 +292,24 @@ protected void burnThroughTransactions(String dbName, String tblName, int num, S
     burnThroughTransactions(dbName, tblName, num, open, aborted, null);
   }
 
-  protected void burnThroughTransactions(String dbName, String tblName, int num, Set<Long> open, Set<Long> aborted, LockRequest lockReq)
+  protected void burnThroughTransactions(String dbName, String tblName, int num, Set<Long> open, Set<Long> aborted,
+      LockRequest lockReq)
       throws MetaException, NoSuchTxnException, TxnAbortedException {
     OpenTxnsResponse rsp = txnHandler.openTxns(new OpenTxnRequest(num, "me", "localhost"));
     AllocateTableWriteIdsRequest awiRqst = new AllocateTableWriteIdsRequest(dbName, tblName);
     awiRqst.setTxnIds(rsp.getTxn_ids());
     AllocateTableWriteIdsResponse awiResp = txnHandler.allocateTableWriteIds(awiRqst);
     int i = 0;
     for (long tid : rsp.getTxn_ids()) {
-      assert(awiResp.getTxnToWriteIds().get(i++).getTxnId() == tid);
-      if(lockReq != null) {
+      assert (awiResp.getTxnToWriteIds().get(i).getTxnId() == tid);
+      ++i;
+      if (lockReq != null) {
         lockReq.setTxnid(tid);
         txnHandler.lock(lockReq);
       }
-      if (aborted != null && aborted.contains(tid)) {
+      if ((aborted != null) && aborted.contains(tid)) {
         txnHandler.abortTxn(new AbortTxnRequest(tid));
-      } else if (open == null || (open != null && !open.contains(tid))) {
+      } else if ((open == null) || !open.contains(tid)) {

Review comment:
       unnecessary brackets




-- 
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: gitbox-unsubscribe@hive.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] vcsomor commented on pull request #2837: HIVE-25716 Fix of flaky test TestCompactionMetrics#testOldestReadyForCleaningAge

Posted by GitBox <gi...@apache.org>.
vcsomor commented on pull request #2837:
URL: https://github.com/apache/hive/pull/2837#issuecomment-987700542


   Root cause was:
   ```java
   Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.OLDEST_READY_FOR_CLEANING_AGE).intValue() >= youngDiff);
   ```
   Output:
   ```
   Now: 1638863282765
   Old start: 1638863245187
   Young start: 1638863263762
   Old diff: 37 (diff 37578)
   Young diff: 19 (19003)
   Age: 18 (which is calculated and rounded inside the runAcidMetricsService)
   ```


-- 
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: gitbox-unsubscribe@hive.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] deniskuzZ commented on a change in pull request #2837: HIVE-25716 Fix of flaky test TestCompactionMetrics#testOldestReadyForCleaningAge

Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #2837:
URL: https://github.com/apache/hive/pull/2837#discussion_r761822308



##########
File path: ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/CompactorTest.java
##########
@@ -292,22 +292,24 @@ protected void burnThroughTransactions(String dbName, String tblName, int num, S
     burnThroughTransactions(dbName, tblName, num, open, aborted, null);
   }
 
-  protected void burnThroughTransactions(String dbName, String tblName, int num, Set<Long> open, Set<Long> aborted, LockRequest lockReq)
+  protected void burnThroughTransactions(String dbName, String tblName, int num, Set<Long> open, Set<Long> aborted,
+      LockRequest lockReq)
       throws MetaException, NoSuchTxnException, TxnAbortedException {
     OpenTxnsResponse rsp = txnHandler.openTxns(new OpenTxnRequest(num, "me", "localhost"));
     AllocateTableWriteIdsRequest awiRqst = new AllocateTableWriteIdsRequest(dbName, tblName);
     awiRqst.setTxnIds(rsp.getTxn_ids());
     AllocateTableWriteIdsResponse awiResp = txnHandler.allocateTableWriteIds(awiRqst);
     int i = 0;
     for (long tid : rsp.getTxn_ids()) {
-      assert(awiResp.getTxnToWriteIds().get(i++).getTxnId() == tid);
-      if(lockReq != null) {
+      assert (awiResp.getTxnToWriteIds().get(i).getTxnId() == tid);
+      ++i;
+      if (lockReq != null) {
         lockReq.setTxnid(tid);
         txnHandler.lock(lockReq);
       }
-      if (aborted != null && aborted.contains(tid)) {
+      if ((aborted != null) && aborted.contains(tid)) {

Review comment:
       unnecessary brackets




-- 
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: gitbox-unsubscribe@hive.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] deniskuzZ commented on a change in pull request #2837: HIVE-25716 Fix of flaky test TestCompactionMetrics#testOldestReadyForCleaningAge

Posted by GitBox <gi...@apache.org>.
deniskuzZ commented on a change in pull request #2837:
URL: https://github.com/apache/hive/pull/2837#discussion_r761811269



##########
File path: ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestCompactionMetrics.java
##########
@@ -178,41 +178,44 @@ public void testInitiatorPerfMetricsDisabled() throws Exception {
   }
 
   @Test
-  @org.junit.Ignore("HIVE-25716")
   public void testOldestReadyForCleaningAge() throws Exception {
     conf.setIntVar(HiveConf.ConfVars.COMPACTOR_MAX_NUM_DELTA, 1);
 
-    long oldStart = System.currentTimeMillis();
-    Table old = newTable("default", "old_rfc", true);
-    Partition oldP = newPartition(old, "part");
+    final String DB_NAME = "default";
+    final String OLD_TABLE_NAME = "old_rfc";
+    final String OLD_PARTITION_NAME = "part";
+    final String YOUNG_TABLE_NAME = "young_rfc";
+    final String YOUNG_PARTITION_NAME = "part";
+
+    Table old = newTable(DB_NAME, OLD_TABLE_NAME, true);
+    Partition oldP = newPartition(old, OLD_PARTITION_NAME);
     addBaseFile(old, oldP, 20L, 20);
     addDeltaFile(old, oldP, 21L, 22L, 2);
     addDeltaFile(old, oldP, 23L, 24L, 2);
-    burnThroughTransactions("default", "old_rfc", 25);
-    CompactionRequest rqst = new CompactionRequest("default", "old_rfc", CompactionType.MINOR);
-    rqst.setPartitionname("ds=part");
-    txnHandler.compact(rqst);
-    startWorker();
+    burnThroughTransactions(DB_NAME, OLD_TABLE_NAME, 25);
+    doCompaction(DB_NAME, OLD_TABLE_NAME, OLD_PARTITION_NAME, CompactionType.MINOR);
+    long oldTableClosestCqCommitTime = System.currentTimeMillis();
 
-    long youngStart = System.currentTimeMillis();
-    Table young = newTable("default", "young_rfc", true);
-    Partition youngP = newPartition(young, "part");
+    Table young = newTable(DB_NAME, YOUNG_TABLE_NAME, true);
+    Partition youngP = newPartition(young, YOUNG_PARTITION_NAME);
     addBaseFile(young, youngP, 20L, 20);
     addDeltaFile(young, youngP, 21L, 22L, 2);
     addDeltaFile(young, youngP, 23L, 24L, 2);
-    burnThroughTransactions("default", "young_rfc", 25);
-    rqst = new CompactionRequest("default", "young_rfc", CompactionType.MINOR);
-    rqst.setPartitionname("ds=part");
-    txnHandler.compact(rqst);
-    startWorker();
+    burnThroughTransactions(DB_NAME, YOUNG_TABLE_NAME, 25);
+    doCompaction(DB_NAME, YOUNG_TABLE_NAME, YOUNG_PARTITION_NAME, CompactionType.MINOR);
+    long youngTableClosestCqCommitTime = System.currentTimeMillis();
+
+    Thread.sleep(1500);

Review comment:
       what's the purpose of 1.5 sec sleep?




-- 
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: gitbox-unsubscribe@hive.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org