impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] CDH-48291: Fix flaky test TestRequestPoolService
Date Thu, 15 Dec 2016 01:17:08 GMT
Matthew Jacobs has posted comments on this change.

Change subject: CDH-48291: Fix flaky test TestRequestPoolService
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5507/1//COMMIT_MSG
Commit Message:

PS1, Line 7: CDH-48291
is this kosher?


http://gerrit.cloudera.org:8080/#/c/5507/1/fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java
File fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java:

PS1, Line 204:     Thread.sleep(10000L + CHECK_INTERVAL_MS +
             :         AllocationFileLoaderService.ALLOC_RELOAD_WAIT_MS);
so the only issue is that this will always sleep for 16sec (assuming the last var is 5sec).
I think to avoid adding a lot of time to tests, we should check every second (or so) up to
this amount of time. This is kinda a pain here because checkModifiedConfigResults() doesn't
lend itself to this model as it's written, but otherwise this is usually going to be a lot
longer than needed.

I haven't looked at the code in a wihle, but maybe it's possible to wait on something in AllocationFileLoaderService?
 That seems best. Or otherwise maybe you catch exceptions from checkModifiedResults and retry
every second up to 15 or 30 seconds at which point we rethrow the exception? That is hackier
but easy.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I71a036d5b1227508e2175808d7e92837ae247d99
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message