----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52631/#review151799 -----------------------------------------------------------
service/src/test/org/apache/hive/service/cli/session/TestSessionManagerMetrics.java (line 106) <https://reviews.apache.org/r/52631/#comment220330> Seems you could run into the following case which will lead to a unwanted failure. Assume 3 or 4 backgroundOperation threads call ready.await() first, then the call in the main thread (line 112) will have 4 or 5 threads merge. Then the main threads get to line 124, seems it will timeout since no threads will call ready.await() any more? Just checked CyclicBarrier doc and not very sure how it should be used actually. Maybe I'm wrong. - Aihua Xu On Oct. 7, 2016, 2 p.m., Marta Kuczora wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52631/ > ----------------------------------------------------------- > > (Updated Oct. 7, 2016, 2 p.m.) > > > Review request for hive, Aihua Xu, Miklos Csanady, Peter Vary, Szehon Ho, and > Barna Zsombor Klara. > > > Bugs: HIVE-14839 > https://issues.apache.org/jira/browse/HIVE-14839 > > > Repository: hive-git > > > Description > ------- > > The TestSessionManagerMetrics fails occasionally with the following error: > org.junit.ComparisonFailure: expected:<[0]> but was:<[1]> > at > org.apache.hive.service.cli.session.TestSessionManagerMetrics.testThreadPoolMetrics(TestSessionManagerMetrics.java:98) > > Changed the synchronization between the tasks a bit to avoid these failures. > With this patch the test would work do the following steps: > - Submit four tasks. > - Wait with the metrics verification, until the first two tasks are running. > This is done by invoking await on the "ready" barrier. > If, for some reason, the tasks are not started within a timeout period, > make the test fail. > - Make the tasks wait until the metrics are checked. > This is done by invoking await on the "completed" barrier. > - Verify the metrics. > - Let the first two tasks complete by breaking the "complete" barrier. > When the first two tasks completed, the remaining two tasks will be removed > from the queue and started. > - Wait until the remaining tasks are running by using the "ready" barrier > again. > Do the metrics check only if they are started to avoid the failures when > the queue size was not 0. > If, for some reason, the tasks are not started within a timeout period, > make the test fail. > - Verify the metrics. > - Let the remaining tasks complete by breaking the "completed" barrier. > > > Diffs > ----- > > > service/src/test/org/apache/hive/service/cli/session/TestSessionManagerMetrics.java > 5511c54 > > Diff: https://reviews.apache.org/r/52631/diff/ > > > Testing > ------- > > The change effects only a unit test. > Ran the test many times locally. > Added random sleeps to the tasks to simulate the delay when they start or > complete and ran the test many times to see if the synch between the tasks is > working correctly. > > > Thanks, > > Marta Kuczora > >
