> On 2010-05-27 16:45:23, Ryan Rawson wrote: > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2701 > > <http://review.hbase.org/r/83/diff/3/?file=690#file690line2701> > > > > I'm thinking perhaps a Static interface could be used instead of the > > getDelegate().currentTimeMillis() that way the individual call sites could > > say: > > > > EnvironmentEdge.currentTimeMillis() > > instead of > > System.currentTimeMillis() > > > > but the static calls would be a simple wrapper around the existing > > getDelegate mechanisms... this would just make the code more explicit and > > easier to follow for future generations. > > > > OTher than that, looking great! Thanks for making all the changes!
Ok sounds good. The EnvironmentEdge is the interface, so I think I'll need to put the static method on the manager. The call would be: EnvironmentEdgeManager.currentTimeMillis() I'm adding a mocking library to the pom (EasyMock) to help with the testing. - Daniel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://review.hbase.org/r/83/#review88 ----------------------------------------------------------- On 2010-05-27 14:51:48, Daniel Ploeg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://review.hbase.org/r/83/ > ----------------------------------------------------------- > > (Updated 2010-05-27 14:51:48) > > > Review request for hbase. > > > Summary > ------- > > HBASE-2578 - Add ability for tests to override server-side timestamp setting > (currentTimeMillis). > The solution in this patch ensures that tests use a different timestamp with > a minimal change to the production code paths. > One question I would like to know is whether the change that was made to > HRegion.FIXED_OVERHEAD would cause any other side effects. > > > This addresses bug HBASE-2578. > http://issues.apache.org/jira/browse/HBASE-2578 > > > Diffs > ----- > > src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java 287cd48 > src/main/java/org/apache/hadoop/hbase/util/DefaultEnvironmentEdge.java > PRE-CREATION > src/main/java/org/apache/hadoop/hbase/util/EnvironmentEdge.java > PRE-CREATION > src/main/java/org/apache/hadoop/hbase/util/EnvironmentEdgeManager.java > PRE-CREATION > src/main/java/org/apache/hadoop/hbase/util/IncrementingEnvironmentEdge.java > PRE-CREATION > src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java d3716d6 > > src/test/java/org/apache/hadoop/hbase/util/EnvironmentEdgeManagerTestHelper.java > PRE-CREATION > src/test/java/org/apache/hadoop/hbase/util/TestDefaultEnvironmentEdge.java > PRE-CREATION > src/test/java/org/apache/hadoop/hbase/util/TestEnvironmentEdgeManager.java > PRE-CREATION > > src/test/java/org/apache/hadoop/hbase/util/TestIncrementingEnvironmentEdge.java > PRE-CREATION > > Diff: http://review.hbase.org/r/83/diff > > > Testing > ------- > > > Thanks, > > Daniel > >
