accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Havanki" <bhava...@clouderagovt.com>
Subject Re: Review Request 17947: ACCUMULO-841 - refactor of randomwalk State
Date Tue, 11 Feb 2014 17:09:43 GMT


> On Feb. 11, 2014, 11:01 a.m., Eric Newton wrote:
> > test/src/main/java/org/apache/accumulo/test/randomwalk/ConfigObjectManager.java,
line 39
> > <https://reviews.apache.org/r/17947/diff/1/?file=482584#file482584line39>
> >
> >     Almost every class can be considered an ObjectManager.  Consider renaming the
class ClientSideObjects or ClientObjects.
> 
> Bill Havanki wrote:
>     Much better names. Will do.
> 
> kturner wrote:
>     Naming stuff sucks.  I was thinking about what the goal of this refactoring is. 
 One reason is to make it easier for someone to write random walk test.   Looking at the existing
code I think one possible source of confusion is that methods intended for use by the framework
itself are public.  Someone trying to write RW test has to spend time figuring this out so
they can ignore those methods.  
>     
>     Maybe rename ClientObjectManager to TestEnvironment (I think this communicates its
purpose better) and add the username and password methods to it.  Looking at the existing
getProperties method on State, its only used by the framework code and holds config for the
framework.   Could possibly make getProperties() package private and add javadoc about it
returning framework config.   There are some other things in state that are not used by test
and could be made package private.
> 
> Josh Elser wrote:
>     Trying to wrap your head around even how the RW Framework gets instantiated, how
Fixtures, Tests and Modules are used, much less the lifetime of State makes it hard to even
understand how the tests run. Any changes to RW that can help make things more obvious would
be awesome.

Good deal. What I'll do here is rework things toward that end, at least for State and the
refactored classes. I'll file a follow-on JIRA to basically demystify randomwalk in a broader
sense, involving Node and Module and Fixture and all that.

Thanks for all the pointers. :)


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17947/#review34188
-----------------------------------------------------------


On Feb. 11, 2014, 10:24 a.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17947/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2014, 10:24 a.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-841
>     https://issues.apache.org/jira/browse/ACCUMULO-841
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> This is a refactoring of the State class used for randomwalk. I tried to balance the
extent of the refactoring with the extent of changes to the tests themselves. So, State is
still a central class, but it delegates to other more focused classes for almost everything
beyond storing just state key/value pairs. Other notes:
> 
> - The node visiting mechanism was unused so it's removed. Module has a maxHops counting
mechanism which does the same thing.
> - The getCredentials() method was only ever used to then get an authentication token,
so the tests now get the token directly, and getCredentials() is gone.
> 
> 
> Diffs
> -----
> 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/ConfigObjectManager.java PRE-CREATION

>   test/src/main/java/org/apache/accumulo/test/randomwalk/ConfigProperties.java PRE-CREATION

>   test/src/main/java/org/apache/accumulo/test/randomwalk/Module.java 25684809daf29644550c05256c7abaf60fc2e75c

>   test/src/main/java/org/apache/accumulo/test/randomwalk/Node.java 1868adedcbb41040d2b0e2e2edff642799f10673

>   test/src/main/java/org/apache/accumulo/test/randomwalk/State.java f4102ab5dbff63e18ba5673126eeb5e646b84771

>   test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTable.java 5a2172d6eddf302fb266c82e174ab235358e9cd6

>   test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTablePerm.java
335ae31dc05ebade9f30187a2ab4d9c79ae205c6 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/security/SecurityFixture.java
30f12fbc57b625489271a3a4a23f8c21b4df9ec1 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/security/SetAuths.java 54bc34aface7428c3149ffbc438cc0be82cee72f

>   test/src/main/java/org/apache/accumulo/test/randomwalk/security/Validate.java 03718ce4e044e985cb309c01c531630cf9222a33

>   test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerify.java
f60fe44c66a6f23242aa442f279ecb86c5162109 
>   test/src/main/java/org/apache/accumulo/test/randomwalk/shard/BulkInsert.java 41acce2575de571fd48fdf5759455be3c5698e33

> 
> Diff: https://reviews.apache.org/r/17947/diff/
> 
> 
> Testing
> -------
> 
> Compile works; ran MultiTable randomwalk, which does run MR jobs to test getMapReduceJars();
also ran Security randomwalk.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message