You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@uniffle.apache.org by ro...@apache.org on 2022/10/12 12:11:04 UTC

[incubator-uniffle] branch branch-0.6 updated (3b8ddc64 -> 177caa50)

This is an automated email from the ASF dual-hosted git repository.

roryqi pushed a change to branch branch-0.6
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git


    from 3b8ddc64 Rename DISCLAIMER to DISCLAIMER-WIP (#258)
     new e9540e4a [MINOR] Fix flaky test (#238)
     new 7088e5f0 Fix Flaky test GetShuffleReportForMultiPartTest (#241)
     new 177caa50 [ISSUE-244] Fix flaky test of CoordinatorGrpcTest.rpcMetricsTest (#256)

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/org/apache/uniffle/test/CoordinatorGrpcTest.java     |  8 +-------
 .../apache/uniffle/test/GetShuffleReportForMultiPartTest.java | 11 ++++++-----
 2 files changed, 7 insertions(+), 12 deletions(-)


[incubator-uniffle] 01/03: [MINOR] Fix flaky test (#238)

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

roryqi pushed a commit to branch branch-0.6
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git

commit e9540e4ad2d1b9f30e5777be9b51650f13dc16f3
Author: roryqi <je...@gmail.com>
AuthorDate: Fri Sep 23 10:14:38 2022 +0800

    [MINOR] Fix flaky test (#238)
    
    ### What changes were proposed in this pull request?
    `org.apache.uniffle.test.GetShuffleReportForMultiPartTest` is flaky
     throws Exception:
    ava.lang.ClassCastException: org.apache.spark.shuffle.RssShuffleManager cannot be cast to org.apache.uniffle.test.GetShuffleReportForMultiPartTest$RssShuffleManagerWrapper
            at org.apache.uniffle.test.GetShuffleReportForMultiPartTest.runTest(GetShuffleReportForMultiPartTest.java:180)
            at org.apache.uniffle.test.SparkIntegrationTestBase.runSparkApp(SparkIntegrationTestBase.java:74)
            at org.apache.uniffle.test.SparkIntegrationTestBase.run(SparkIntegrationTestBase.java:52)
            at org.apache.uniffle.test.GetShuffleReportForMultiPartTest.resultCompareTest(GetShuffleReportForMultiPartTest.java:141)
    
    ### Why are the changes needed?
    Fix flaky test
    
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    GA passed
    
    Co-authored-by: roryqi <ro...@tencent.com>
---
 .../java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/integration-test/spark3/src/test/java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java b/integration-test/spark3/src/test/java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java
index f719c96d..899970ce 100644
--- a/integration-test/spark3/src/test/java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java
+++ b/integration-test/spark3/src/test/java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java
@@ -175,7 +175,8 @@ public class GetShuffleReportForMultiPartTest extends SparkIntegrationTestBase {
       i++;
     }
     SparkConf conf = spark.sparkContext().conf();
-    if (!conf.get("spark.shuffle.manager", "").isEmpty()) {
+    if (conf.get("spark.shuffle.manager", "")
+        .equals("org.apache.uniffle.test.GetShuffleReportForMultiPartTest$RssShuffleManagerWrapper")) {
       RssShuffleManagerWrapper mockRssShuffleManager =
           (RssShuffleManagerWrapper) spark.sparkContext().env().shuffleManager();
       int expectRequestNum = mockRssShuffleManager.getShuffleIdToPartitionNum().values().stream()


[incubator-uniffle] 02/03: Fix Flaky test GetShuffleReportForMultiPartTest (#241)

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

roryqi pushed a commit to branch branch-0.6
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git

commit 7088e5f00923e1cb631180b7154a93fa365f3a00
Author: Xianming Lei <31...@users.noreply.github.com>
AuthorDate: Fri Sep 23 22:18:58 2022 +0800

    Fix Flaky test GetShuffleReportForMultiPartTest (#241)
    
    ### What changes were proposed in this pull request?
    ```
    org.opentest4j.AssertionFailedError: expected: <0> but was: <-218>
            at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
            at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
            at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
            at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
            at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:527)
            at org.apache.uniffle.test.GetShuffleReportForMultiPartTest.validateRequestCount(GetShuffleReportForMultiPartTest.java:198)
            at org.apache.uniffle.test.GetShuffleReportForMultiPartTest.runTest(GetShuffleReportForMultiPartTest.java:185)
            at org.apache.uniffle.test.SparkIntegrationTestBase.runSparkApp(SparkIntegrationTestBase.java:74)
            at org.apache.uniffle.test.SparkIntegrationTestBase.run(SparkIntegrationTestBase.java:59)
            at org.apache.uniffle.test.GetShuffleReportForMultiPartTest.resultCompareTest(GetShuffleReportForMultiPartTest.java:141)
            at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
            at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
            at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
            at java.lang.reflect.Method.invoke(Method.java:498)
            at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
            at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
            at
    ```
    
    ### Why are the changes needed?
    Fix flaky test
    
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    UT passed
    
    Co-authored-by: leixianming <le...@didiglobal.com>
---
 .../org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/integration-test/spark3/src/test/java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java b/integration-test/spark3/src/test/java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java
index 899970ce..ec7a7c93 100644
--- a/integration-test/spark3/src/test/java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java
+++ b/integration-test/spark3/src/test/java/org/apache/uniffle/test/GetShuffleReportForMultiPartTest.java
@@ -182,17 +182,17 @@ public class GetShuffleReportForMultiPartTest extends SparkIntegrationTestBase {
       int expectRequestNum = mockRssShuffleManager.getShuffleIdToPartitionNum().values().stream()
           .mapToInt(x -> x.get()).sum();
       // Validate getShuffleResultForMultiPart is correct before return result
-      validateRequestCount(expectRequestNum * replicateRead);
+      validateRequestCount(spark.sparkContext().applicationId(), expectRequestNum * replicateRead);
     }
     return map;
   }
 
-  public void validateRequestCount(int expectRequestNum) {
+  public void validateRequestCount(String appId, int expectRequestNum) {
     for (ShuffleServer shuffleServer : shuffleServers) {
       MockedShuffleServerGrpcService service = ((MockedGrpcServer) shuffleServer.getServer()).getService();
       Map<String, Map<Integer, AtomicInteger>> serviceRequestCount = service.getShuffleIdToPartitionRequest();
-      int requestNum = serviceRequestCount.entrySet().stream().flatMap(x -> x.getValue().values()
-           .stream()).mapToInt(AtomicInteger::get).sum();
+      int requestNum = serviceRequestCount.entrySet().stream().filter(x -> x.getKey().startsWith(appId))
+          .flatMap(x -> x.getValue().values().stream()).mapToInt(AtomicInteger::get).sum();
       expectRequestNum -= requestNum;
     }
     assertEquals(0, expectRequestNum);


[incubator-uniffle] 03/03: [ISSUE-244] Fix flaky test of CoordinatorGrpcTest.rpcMetricsTest (#256)

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

roryqi pushed a commit to branch branch-0.6
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git

commit 177caa50a425a8d816590fd16e3d3a4add50bfdb
Author: Junfan Zhang <zu...@apache.org>
AuthorDate: Tue Oct 11 11:59:13 2022 +0800

    [ISSUE-244] Fix flaky test of CoordinatorGrpcTest.rpcMetricsTest (#256)
    
    ### What changes were proposed in this pull request?
    [ISSUE-244] Fix flaky test of CoordinatorGrpcTest.rpcMetricsTest
    
    ### Why are the changes needed?
    1. The gauge metric of `HEARTBEAT_METHOD` is hard to meansure due to async sending at background. So remove it
    2. The gauge metric of `GET_SHUFFLE_ASSIGNMENTS_METHOD` may be not called `descCounter` when rpc finished, so remove it
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    Dont need
---
 .../test/java/org/apache/uniffle/test/CoordinatorGrpcTest.java    | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/integration-test/common/src/test/java/org/apache/uniffle/test/CoordinatorGrpcTest.java b/integration-test/common/src/test/java/org/apache/uniffle/test/CoordinatorGrpcTest.java
index c09067ea..3cc79632 100644
--- a/integration-test/common/src/test/java/org/apache/uniffle/test/CoordinatorGrpcTest.java
+++ b/integration-test/common/src/test/java/org/apache/uniffle/test/CoordinatorGrpcTest.java
@@ -245,13 +245,10 @@ public class CoordinatorGrpcTest extends CoordinatorTestBase {
   public void rpcMetricsTest() throws Exception {
     double oldValue = coordinators.get(0).getGrpcMetrics().getCounterMap()
         .get(CoordinatorGrpcMetrics.HEARTBEAT_METHOD).get();
-    CoordinatorTestUtils.waitForRegister(coordinatorClient,2);
+    CoordinatorTestUtils.waitForRegister(coordinatorClient, 2);
     double newValue = coordinators.get(0).getGrpcMetrics().getCounterMap()
         .get(CoordinatorGrpcMetrics.HEARTBEAT_METHOD).get();
     assertTrue(newValue - oldValue > 1);
-    assertEquals(0,
-        coordinators.get(0).getGrpcMetrics().getGaugeMap()
-            .get(CoordinatorGrpcMetrics.HEARTBEAT_METHOD).get(), 0.5);
 
     String appId = "rpcMetricsTest";
     RssGetShuffleAssignmentsRequest request = new RssGetShuffleAssignmentsRequest(
@@ -263,9 +260,6 @@ public class CoordinatorGrpcTest extends CoordinatorTestBase {
     newValue = coordinators.get(0).getGrpcMetrics().getCounterMap()
         .get(CoordinatorGrpcMetrics.GET_SHUFFLE_ASSIGNMENTS_METHOD).get();
     assertEquals(oldValue + 1, newValue, 0.5);
-    assertEquals(0,
-        coordinators.get(0).getGrpcMetrics().getGaugeMap()
-            .get(CoordinatorGrpcMetrics.GET_SHUFFLE_ASSIGNMENTS_METHOD).get(), 0.5);
 
     double connectionSize = coordinators.get(0)
         .getGrpcMetrics().getGaugeMap().get(GRCP_SERVER_CONNECTION_NUMBER_KEY).get();