accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 28214: Decouple MiniAccumuloCluster from integration tests and introduce StandaloneAccumuloCluster
Date Mon, 24 Nov 2014 23:17:50 GMT


> On Nov. 19, 2014, 5 p.m., Josh Elser wrote:
> > test/src/test/java/org/apache/accumulo/test/Accumulo3047IT.java, line 77
> > <https://reviews.apache.org/r/28214/diff/3/?file=769778#file769778line77>
> >
> >     This turns out to be a really common pattern that cropped up across the test
code. Providing some mechanism to automatically do this in AccumuloClusterIT would be nice.

Will do in follow-on.


> On Nov. 19, 2014, 5 p.m., Josh Elser wrote:
> > test/src/test/java/org/apache/accumulo/harness/conf/AccumuloStandaloneClusterConfiguration.java,
line 32
> > <https://reviews.apache.org/r/28214/diff/3/?file=769775#file769775line32>
> >
> >     Will need to add some more pieces here to run against an instance with SSL enabled.

Will do in follow-on.


> On Nov. 19, 2014, 5 p.m., Josh Elser wrote:
> > test/src/test/java/org/apache/accumulo/harness/AccumuloClusterIT.java, line 197
> > <https://reviews.apache.org/r/28214/diff/3/?file=769767#file769767line197>
> >
> >     I wasn't really sure what should be done here. It helps keep the code more backwards
compatible but I don't like adding implementation specifics to this class.
> >     
> >     It might be possible to push this down to the AccumuloCluster implementation.
The worry for a standalone instance is that we'd want it returned to it's original state after
the test. To do that, we'd also have to add a teardown hook to the AccumuloCluster implementation
that this class could invoke.

Can address if better approach comes up later.


> On Nov. 19, 2014, 5 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java,
line 290
> > <https://reviews.apache.org/r/28214/diff/3/?file=769757#file769757line290>
> >
> >     This is rather brittle because it expects the tests to be running on a properly
configured host for the instance that you're connecting to. Acceptable for a first pass?

Will address when it becomes an issue.


> On Nov. 19, 2014, 5 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/cluster/ClusterControl.java, line
46
> > <https://reviews.apache.org/r/28214/diff/3/?file=769752#file769752line46>
> >
> >     Are the variants that accept a hostname even worthwhile? The caller would have
to know where things are running which is hard to get at presently.

Can remove later if desired, not intended for client use anyways.


> On Nov. 19, 2014, 5 p.m., Josh Elser wrote:
> > minicluster/src/main/java/org/apache/accumulo/cluster/ClusterControl.java, line
31
> > <https://reviews.apache.org/r/28214/diff/3/?file=769752#file769752line31>
> >
> >     Not sure if it's better to accept `String[] args` or `String... args`. MiniAccumuloClusterImpl's
`exec` method did accept the varargs variant.

6 of one, half-dozen of another.


> On Nov. 19, 2014, 5 p.m., Josh Elser wrote:
> > test/src/test/java/org/apache/accumulo/test/functional/RestartIT.java, line 122
> > <https://reviews.apache.org/r/28214/diff/3/?file=769843#file769843line122>
> >
> >     This is a big downside of implementing ClusterControl how I did. It's easy to
just run something and get the exit code, but it's difficult to spawn a background process
and wait for it to return.
> >     
> >     Is it worth it to provide both of these methods?

I haven't hit a test that isn't satisfied by getting the return code and stdout or doing the
above. Most aren't really forking off anyways.


- Josh


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


On Nov. 21, 2014, 8:40 p.m., Josh Elser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28214/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2014, 8:40 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3167
>     https://issues.apache.org/jira/browse/ACCUMULO-3167
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> We have a large number of good tests which are only capable of being run against a MiniAccumuloCluster.
This is undesirable because it limits our tests to the scope of what MiniAccumuloCluster provides,
or implements as a standalone cluster does. An accurate test environment would be a true deployment
with distributed HDFS and ZooKeper instances that back a distributed Accumulo instance. This
patch introduces a StandaloneAccumuloCluster, in addition to a few other new interfaces (ClusterControl)
which help support the necessary functionality. The StandaloneAccumuloCluster is the "MiniAccumuloCluster"
counterpart to a distributed cluster.
> 
> Given the StandaloneAccumuloCluster, many of the integration tests need to be rewritten
in such a way that support both a MiniAccumuloCluster and the Standalone cluster. While being
a painful set of changes, this does help generalize some of the tests and conform them to
some best practices to simplify things.
> 
> I also nuked some initial interfaces which I initially stubbed out because they turned
out not being useful.
> 
> 
> Diffs
> -----
> 
>   minicluster/src/main/java/org/apache/accumulo/cluster/AccumuloCluster.java c982de0

>   minicluster/src/main/java/org/apache/accumulo/cluster/AccumuloClusters.java 50cb9db

>   minicluster/src/main/java/org/apache/accumulo/cluster/AccumuloConfig.java 0df2348 
>   minicluster/src/main/java/org/apache/accumulo/cluster/ClusterControl.java PRE-CREATION

>   minicluster/src/main/java/org/apache/accumulo/cluster/RemoteShell.java PRE-CREATION

>   minicluster/src/main/java/org/apache/accumulo/cluster/RemoteShellOptions.java PRE-CREATION

>   minicluster/src/main/java/org/apache/accumulo/cluster/package-info.java f1b649d 
>   minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneAccumuloCluster.java
PRE-CREATION 
>   minicluster/src/main/java/org/apache/accumulo/cluster/standalone/StandaloneClusterControl.java
PRE-CREATION 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/MiniAccumuloCluster.java
3e8c5a0 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/ServerType.java 3590a20 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterControl.java
PRE-CREATION 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImpl.java
7283c19 
>   minicluster/src/main/java/org/apache/accumulo/minicluster/impl/MiniAccumuloConfigImpl.java
2d7103e 
>   minicluster/src/test/java/org/apache/accumulo/cluster/AccumuloClustersTest.java e368240

>   minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterStartStopTest.java
2031b11 
>   minicluster/src/test/java/org/apache/accumulo/minicluster/impl/MiniAccumuloClusterImplTest.java
b19d289 
>   server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java 55548e3

>   test/src/main/java/org/apache/accumulo/test/TestMultiTableIngest.java 16f0b3f 
>   test/src/test/java/org/apache/accumulo/harness/AccumuloClusterIT.java PRE-CREATION

>   test/src/test/java/org/apache/accumulo/harness/AccumuloIT.java PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/harness/MiniClusterConfigurationCallback.java
PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/harness/MiniClusterHarness.java PRE-CREATION

>   test/src/test/java/org/apache/accumulo/harness/SharedMiniClusterIT.java PRE-CREATION

>   test/src/test/java/org/apache/accumulo/harness/conf/AccumuloClusterConfiguration.java
PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/harness/conf/AccumuloClusterPropertyConfiguration.java
PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/harness/conf/AccumuloMiniClusterConfiguration.java
PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/harness/conf/AccumuloStandaloneClusterConfiguration.java
PRE-CREATION 
>   test/src/test/java/org/apache/accumulo/test/Accumulo3010IT.java 791b1d5 
>   test/src/test/java/org/apache/accumulo/test/Accumulo3030IT.java 3512e4a 
>   test/src/test/java/org/apache/accumulo/test/Accumulo3047IT.java 70e1c30 
>   test/src/test/java/org/apache/accumulo/test/AuditMessageIT.java 49b5d70 
>   test/src/test/java/org/apache/accumulo/test/BatchWriterIT.java ca72e7a 
>   test/src/test/java/org/apache/accumulo/test/CleanWalIT.java d0bfe3f 
>   test/src/test/java/org/apache/accumulo/test/ConditionalWriterIT.java 8406570 
>   test/src/test/java/org/apache/accumulo/test/ConfigurableMajorCompactionIT.java 899b41b

>   test/src/test/java/org/apache/accumulo/test/DeleteRowsIT.java ff67e89 
>   test/src/test/java/org/apache/accumulo/test/ExistingMacIT.java ec72281 
>   test/src/test/java/org/apache/accumulo/test/ImportExportIT.java a48ed9d 
>   test/src/test/java/org/apache/accumulo/test/InterruptibleScannersIT.java PRE-CREATION

>   test/src/test/java/org/apache/accumulo/test/KeyValueEqualityTest.java 1302b23 
>   test/src/test/java/org/apache/accumulo/test/MetaConstraintRetryIT.java b3c3640 
>   test/src/test/java/org/apache/accumulo/test/MetaSplitIT.java 50a1446 
>   test/src/test/java/org/apache/accumulo/test/MultiTableBatchWriterIT.java 484c048 
>   test/src/test/java/org/apache/accumulo/test/NamespacesIT.java 8188deb 
>   test/src/test/java/org/apache/accumulo/test/NoMutationRecoveryIT.java 87ad1a3 
>   test/src/test/java/org/apache/accumulo/test/ShellServerIT.java 4457e70 
>   test/src/test/java/org/apache/accumulo/test/SplitRecoveryIT.java 96d3a1a 
>   test/src/test/java/org/apache/accumulo/test/TableConfigurationUpdateIT.java 0d9a211

>   test/src/test/java/org/apache/accumulo/test/TableOperationsIT.java bb12279 
>   test/src/test/java/org/apache/accumulo/test/VolumeIT.java d5c940d 
>   test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java 22e46ff 
>   test/src/test/java/org/apache/accumulo/test/functional/AccumuloInputFormatIT.java ad84960

>   test/src/test/java/org/apache/accumulo/test/functional/AddSplitIT.java 05de342 
>   test/src/test/java/org/apache/accumulo/test/functional/BadIteratorMincIT.java 9c4492e

>   test/src/test/java/org/apache/accumulo/test/functional/BalanceInPresenceOfOfflineTableIT.java
887aee4 
>   test/src/test/java/org/apache/accumulo/test/functional/BatchScanSplitIT.java 688a326

>   test/src/test/java/org/apache/accumulo/test/functional/BatchWriterFlushIT.java 465936e

>   test/src/test/java/org/apache/accumulo/test/functional/BigRootTabletIT.java b021c3a

>   test/src/test/java/org/apache/accumulo/test/functional/BinaryIT.java 9c0edaa 
>   test/src/test/java/org/apache/accumulo/test/functional/BinaryStressIT.java a60c2d5

>   test/src/test/java/org/apache/accumulo/test/functional/BloomFilterIT.java 8f6b830 
>   test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java 831dcd4 
>   test/src/test/java/org/apache/accumulo/test/functional/ChaoticBalancerIT.java 8afb3d2

>   test/src/test/java/org/apache/accumulo/test/functional/ClassLoaderIT.java d71819e 
>   test/src/test/java/org/apache/accumulo/test/functional/CleanUpIT.java 79bbb90 
>   test/src/test/java/org/apache/accumulo/test/functional/CloneTestIT.java 505dd5a 
>   test/src/test/java/org/apache/accumulo/test/functional/CombinerIT.java 69f9134 
>   test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java 92bd714 
>   test/src/test/java/org/apache/accumulo/test/functional/ConfigurableMacIT.java 9185e1b

>   test/src/test/java/org/apache/accumulo/test/functional/ConstraintIT.java 7e5944e 
>   test/src/test/java/org/apache/accumulo/test/functional/CreateAndUseIT.java 5b5249b

>   test/src/test/java/org/apache/accumulo/test/functional/CreateManyScannersIT.java aed38e8

>   test/src/test/java/org/apache/accumulo/test/functional/CredentialsIT.java 8e2e1e0 
>   test/src/test/java/org/apache/accumulo/test/functional/DeleteEverythingIT.java 0578ef4

>   test/src/test/java/org/apache/accumulo/test/functional/DeleteIT.java 3510fbd 
>   test/src/test/java/org/apache/accumulo/test/functional/DeleteRowsIT.java 4b7d664 
>   test/src/test/java/org/apache/accumulo/test/functional/DeleteRowsSplitIT.java d35ba9f

>   test/src/test/java/org/apache/accumulo/test/functional/DeleteTableDuringSplitIT.java
a0bff64 
>   test/src/test/java/org/apache/accumulo/test/functional/DynamicThreadPoolsIT.java 87497b9

>   test/src/test/java/org/apache/accumulo/test/functional/FateStarvationIT.java 4d75a16

>   test/src/test/java/org/apache/accumulo/test/functional/LargeRowIT.java d77d060 
>   test/src/test/java/org/apache/accumulo/test/functional/LogicalTimeIT.java 6aec7cd 
>   test/src/test/java/org/apache/accumulo/test/functional/MasterAssignmentIT.java 46f6b23

>   test/src/test/java/org/apache/accumulo/test/functional/MasterFailoverIT.java 218d65e

>   test/src/test/java/org/apache/accumulo/test/functional/MaxOpenIT.java 2649890 
>   test/src/test/java/org/apache/accumulo/test/functional/MergeIT.java c264dfe 
>   test/src/test/java/org/apache/accumulo/test/functional/MetadataIT.java bd0282d 
>   test/src/test/java/org/apache/accumulo/test/functional/MetadataMaxFiles.java 6b8d9b3

>   test/src/test/java/org/apache/accumulo/test/functional/MetadataMaxFilesIT.java PRE-CREATION

>   test/src/test/java/org/apache/accumulo/test/functional/PermissionsIT.java d51dcbb 
>   test/src/test/java/org/apache/accumulo/test/functional/ReadWriteIT.java bc9ab4a 
>   test/src/test/java/org/apache/accumulo/test/functional/RecoveryWithEmptyRFileIT.java
814dd85 
>   test/src/test/java/org/apache/accumulo/test/functional/RenameIT.java 8cbe84f 
>   test/src/test/java/org/apache/accumulo/test/functional/RestartIT.java e4b9c5a 
>   test/src/test/java/org/apache/accumulo/test/functional/RestartStressIT.java d8c2804

>   test/src/test/java/org/apache/accumulo/test/functional/RowDeleteIT.java 4dbd912 
>   test/src/test/java/org/apache/accumulo/test/functional/ScanIteratorIT.java 189a55c

>   test/src/test/java/org/apache/accumulo/test/functional/ScanRangeIT.java 90b881c 
>   test/src/test/java/org/apache/accumulo/test/functional/ScanSessionTimeOutIT.java 3547b68

>   test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java cbd1290 
>   test/src/test/java/org/apache/accumulo/test/functional/ServerSideErrorIT.java d765b16

>   test/src/test/java/org/apache/accumulo/test/functional/SimpleMacIT.java a4e6647 
>   test/src/test/java/org/apache/accumulo/test/functional/SparseColumnFamilyIT.java 0b63d01

>   test/src/test/java/org/apache/accumulo/test/functional/SplitIT.java 6203523 
>   test/src/test/java/org/apache/accumulo/test/functional/SslIT.java a14795c 
>   test/src/test/java/org/apache/accumulo/test/functional/StartIT.java 82278af 
>   test/src/test/java/org/apache/accumulo/test/functional/TableIT.java 832ec60 
>   test/src/test/java/org/apache/accumulo/test/functional/TabletIT.java fccc79f 
>   test/src/test/java/org/apache/accumulo/test/functional/TimeoutIT.java 4dc72e0 
>   test/src/test/java/org/apache/accumulo/test/functional/VisibilityIT.java f2460cf 
>   test/src/test/java/org/apache/accumulo/test/functional/WriteAheadLogIT.java af6eca5

>   test/src/test/java/org/apache/accumulo/test/functional/WriteLotsIT.java 214fc2f 
> 
> Diff: https://reviews.apache.org/r/28214/diff/
> 
> 
> Testing
> -------
> 
> Haven't had a 100% IT pass rate yet (last run was about 95% pass rate), but I wanted
to get the code up and have some eyes on it sooner than later.
> 
> If you want to try it out, you configure it using properties on the maven command line.
For example,  -Daccumulo.it.cluster.type=STANDALONE -Daccumulo.it.cluster.standalone.principal=root
-Daccumulo.it.cluster.standalone.password=password -Daccumulo.it.cluster.standalone.instance.name=accumulo
-Daccumulo.it.cluster.standalone.zookeepers=localhost. This would use a "real" instance with
the expected connection information. Alternatively, you can make a properties file on the
local filesystem and specify that file directly using -Daccumulo.it.properties=/path/to/file.properties.
Properties on the command line will override those specified in the property if you provide
the same in more than one place.
> 
> 
> Thanks,
> 
> Josh Elser
> 
>


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