You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jason Fehr (Code Review)" <ge...@cloudera.org> on 2023/02/24 21:34:18 UTC

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Jason Fehr has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19536


Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................

IMPALA-11945: Fix Flaky Test in JwtHttpTest

The JwtHttpTest.testJwtAuthWithUntrustedJwksHttpsUrl test has been
flaky in the impala-asf-master-core Jenkins build. This build was
executed twice with this test passing in one run and failing in the
other.

This test and another test named
testJwtAuthWithTrustedJwksHttpsUrlInvalidCN both follow the same
pattern. They attempt to start up a cluster with incorrectly
configured Impala daemons. Then, the Impala coordinator's log file
is searched to ensure the coordinator failed to start up for the
expected reason.

The theory for the test flakiness is that the Impala coordinator logs
did not finish writing to disk before they were checked for the
expected startup failure message. The evidence backing this theory
is that all expected log messages were present in the log file except
for the final log line containing the error message.

Three changes were made to fix the flakiness
1. The tests where log files are search for startup failure messages
   now use a single coordinator. Only a single coordinator is needed
   thus this change eliminates potential issues caused by multiple
   coordinators.
2. The tests sleep 5 seconds before searching the log files to give
   time for the logs to be fully written to disk.
3. Log buffering in the Impala daemons was turned off by setting the
   logbuflevel startup flag to -1 which turns off in-memory log
   buffering and writes the logs directly to disk.

Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
---
M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
1 file changed, 37 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/19536/4
-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 01:06:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19536/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19536/4//COMMIT_MSG@9
PS4, Line 9: The JwtHttpTest.testJwtAuthWithUntrustedJwksHttpsUrl
> nit: mention the IMPALA-11922 in commit message.
done


http://gerrit.cloudera.org:8080/#/c/19536/4/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
File fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java:

http://gerrit.cloudera.org:8080/#/c/19536/4/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@442
PS4, Line 442:     // check in the impalad logs that the server startup failed for the expected reason
> Nit: We could avoid the sleep by looping a few times retrying the readAllLi
fixing to eliminate the 5 second sleep and establish a better pattern



-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 00:08:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19536/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19536/4//COMMIT_MSG@9
PS4, Line 9: The JwtHttpTest.testJwtAuthWithUntrustedJwksHttpsUrl
nit: mention the IMPALA-11922 in commit message.
In case we backport IMPALA-11922 somewhere else, we'll remember to include IMPALA-11945 as well.


http://gerrit.cloudera.org:8080/#/c/19536/4/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
File fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java:

http://gerrit.cloudera.org:8080/#/c/19536/4/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@416
PS4, Line 416: testJwtAuthWithUntrustedJwksHttpsUrl
Can you try loop this test in your local machine? just to make sure it is not flaky anymore. Maybe like this from $IMPALA_HOME:

(pushd fe && for i in {1..100}; do mvn -fae test -Dtest=JwtHttpTest#testJwtAuthWithUntrustedJwksHttpsUrl; done)



-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Feb 2023 22:27:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 5: Code-Review+1

LGTM, I'll let Riza take this to +2


-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 00:22:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................

IMPALA-11945: Fix Flaky Test in JwtHttpTest

The JwtHttpTest.testJwtAuthWithUntrustedJwksHttpsUrl test has been
flaky in the impala-asf-master-core Jenkins build. This build was
executed twice with this test passing in one run and failing in the
other.

This test and another test named
testJwtAuthWithTrustedJwksHttpsUrlInvalidCN both follow the same
pattern. They attempt to start up a cluster with incorrectly
configured Impala daemons. Then, the Impala coordinator's log file
is searched to ensure the coordinator failed to start up for the
expected reason.  Both these tests were introduced by IMPALA-11922.

The theory for the test flakiness is that the Impala coordinator logs
did not finish writing to disk before they were checked for the
expected startup failure message. The evidence backing this theory
is that all expected log messages were present in the log file except
for the final log line containing the error message.

Three changes were made to fix the flakiness
1. The tests where log files are search for startup failure messages
   now use a single coordinator. Only a single coordinator is needed
   thus this change eliminates potential issues caused by multiple
   coordinators.
2. The tests sleep 5 seconds before searching the log files to give
   time for the logs to be fully written to disk.
3. Log buffering in the Impala daemons was turned off by setting the
   logbuflevel startup flag to -1 which turns off in-memory log
   buffering and writes the logs directly to disk.

Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Reviewed-on: http://gerrit.cloudera.org:8080/19536
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
1 file changed, 65 insertions(+), 18 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 8
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 6: Code-Review+2

Looks good, thanks!
Carry Andrew's +1.


-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 01:05:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19536/5/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
File fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java:

http://gerrit.cloudera.org:8080/#/c/19536/5/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@443
PS5, Line 443:     List<String> logLines = null;
             :     Matcher<Iterable<? super String>> m = hasItem(containsString(expectedErrString));
             : 
             :     // writing logs to disk may take some time, try a few times to search for the
             :     // expected error in the log
             :     for (int i=0; i<10; i++) {
             :       logLines = Files.readAllLines(logDir.resolve("impalad.ERROR"));
             :       if (m.matches(logLines)) {
             :         break;
             :       }
             :       Thread.sleep(250);
             :     }
             : 
             :     // runs the matcher one more time to ensure a descriptive failure message is
             :     // generated if the assert fails
> nit: This code is duplicated in two places. Maybe put this into its own met
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 01:04:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12439/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Feb 2023 21:54:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 7: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 06:13:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................

IMPALA-11945: Fix Flaky Test in JwtHttpTest

The JwtHttpTest.testJwtAuthWithUntrustedJwksHttpsUrl test has been
flaky in the impala-asf-master-core Jenkins build. This build was
executed twice with this test passing in one run and failing in the
other.

This test and another test named
testJwtAuthWithTrustedJwksHttpsUrlInvalidCN both follow the same
pattern. They attempt to start up a cluster with incorrectly
configured Impala daemons. Then, the Impala coordinator's log file
is searched to ensure the coordinator failed to start up for the
expected reason.  Both these tests were introduced by IMPALA-11922.

The theory for the test flakiness is that the Impala coordinator logs
did not finish writing to disk before they were checked for the
expected startup failure message. The evidence backing this theory
is that all expected log messages were present in the log file except
for the final log line containing the error message.

Three changes were made to fix the flakiness
1. The tests where log files are search for startup failure messages
   now use a single coordinator. Only a single coordinator is needed
   thus this change eliminates potential issues caused by multiple
   coordinators.
2. The tests sleep 5 seconds before searching the log files to give
   time for the logs to be fully written to disk.
3. Log buffering in the Impala daemons was turned off by setting the
   logbuflevel startup flag to -1 which turns off in-memory log
   buffering and writes the logs directly to disk.

Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
---
M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
1 file changed, 65 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/19536/6
-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................

IMPALA-11945: Fix Flaky Test in JwtHttpTest

The JwtHttpTest.testJwtAuthWithUntrustedJwksHttpsUrl test has been
flaky in the impala-asf-master-core Jenkins build. This build was
executed twice with this test passing in one run and failing in the
other.

This test and another test named
testJwtAuthWithTrustedJwksHttpsUrlInvalidCN both follow the same
pattern. They attempt to start up a cluster with incorrectly
configured Impala daemons. Then, the Impala coordinator's log file
is searched to ensure the coordinator failed to start up for the
expected reason.  Both these tests were introduced by IMPALA-11922.

The theory for the test flakiness is that the Impala coordinator logs
did not finish writing to disk before they were checked for the
expected startup failure message. The evidence backing this theory
is that all expected log messages were present in the log file except
for the final log line containing the error message.

Three changes were made to fix the flakiness
1. The tests where log files are search for startup failure messages
   now use a single coordinator. Only a single coordinator is needed
   thus this change eliminates potential issues caused by multiple
   coordinators.
2. The tests sleep 5 seconds before searching the log files to give
   time for the logs to be fully written to disk.
3. Log buffering in the Impala daemons was turned off by setting the
   logbuflevel startup flag to -1 which turns off in-memory log
   buffering and writes the logs directly to disk.

Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
---
M fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
1 file changed, 61 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/19536/5
-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 5:

(1 comment)

Thanks Jason, just have 1 more nit.

http://gerrit.cloudera.org:8080/#/c/19536/5/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
File fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java:

http://gerrit.cloudera.org:8080/#/c/19536/5/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@443
PS5, Line 443:     List<String> logLines = null;
             :     Matcher<Iterable<? super String>> m = hasItem(containsString(expectedErrString));
             : 
             :     // writing logs to disk may take some time, try a few times to search for the
             :     // expected error in the log
             :     for (int i=0; i<10; i++) {
             :       logLines = Files.readAllLines(logDir.resolve("impalad.ERROR"));
             :       if (m.matches(logLines)) {
             :         break;
             :       }
             :       Thread.sleep(250);
             :     }
             : 
             :     // runs the matcher one more time to ensure a descriptive failure message is
             :     // generated if the assert fails
nit: This code is duplicated in two places. Maybe put this into its own method?



-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 00:16:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19536/4/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
File fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java:

http://gerrit.cloudera.org:8080/#/c/19536/4/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@416
PS4, Line 416: testJwtAuthWithUntrustedJwksHttpsUrl
> Can you try loop this test in your local machine? just to make sure it is n
good idea, I ran the test 100 times and it did not fail once



-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 00:09:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12443/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 6
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 01:25:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9072/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 7
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 01:06:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 4:

(1 comment)

I have a suggestion which you can push back on if you like

http://gerrit.cloudera.org:8080/#/c/19536/4/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java
File fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java:

http://gerrit.cloudera.org:8080/#/c/19536/4/fe/src/test/java/org/apache/impala/customcluster/JwtHttpTest.java@442
PS4, Line 442:     Thread.sleep(5000);
Nit: We could avoid the sleep by looping a few times retrying the readAllLines+containsString with a short sleep after each try.
Impala has a LOT of tests and we want to try and keep them running as fast as possible. So 10 extra seconds of sleep may seem like not much, but it would be better to establish a pattern that doesn't involve sleeping in case other people copy and paste the test,



-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 4
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Feb 2023 22:13:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11945: Fix Flaky Test in JwtHttpTest

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19536 )

Change subject: IMPALA-11945: Fix Flaky Test in JwtHttpTest
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/12441/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19536
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I26d8c2cffa7bc095b81eb572709f55b04ac1fa67
Gerrit-Change-Number: 19536
Gerrit-PatchSet: 5
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Sat, 25 Feb 2023 00:28:59 +0000
Gerrit-HasComments: No