> On July 1, 2014, 4:32 p.m., Josh Elser wrote: > > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java, line > > 88 > > <https://reviews.apache.org/r/23200/diff/1/?file=621351#file621351line88> > > > > Would be nice if we could skip this check on the first iteration. > > Sean Busbey wrote: > We almost certainly won't have passed the timeout interval, why add the > extra state tracking? > > Josh Elser wrote: > To the same argument, why call to get the time again when we almost > certainly haven't passed the timeout interval?
You could add another lastCheckedTime which is initialized to startTime outside the loop. Then, at the end of the loop, you could set lastCheckedTime equal to System.currentTimeMillis(). That would make me happy and alleviate some extra boolean. - Josh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23200/#review47081 ----------------------------------------------------------- On July 1, 2014, 4:23 p.m., Sean Busbey wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23200/ > ----------------------------------------------------------- > > (Updated July 1, 2014, 4:23 p.m.) > > > Review request for accumulo and Mike Drob. > > > Bugs: ACCUMULO-2967 > https://issues.apache.org/jira/browse/ACCUMULO-2967 > > > Repository: accumulo > > > Description > ------- > > - Tests that we get an exception within a reasonable bound > - Changes timeout condition to check prior to place where an exception can be > thrown. > > > Diffs > ----- > > fate/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java > 205ff01809141f4fcabf860dbb93469207ac2842 > fate/src/test/java/org/apache/accumulo/fate/zookeeper/ZooSessionTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/23200/diff/ > > > Testing > ------- > > prior to change, given test fails on timeout (tested up to a 5 minute wait). > post change test passes. > > > Thanks, > > Sean Busbey > >
