> On July 10, 2014, 1:42 a.m., Gregory Chanan wrote:
> > trunk/solr/core/src/java/org/apache/solr/cloud/OverseerAutoReplicaFailoverThread.java,
> >  line 82
> > <https://reviews.apache.org/r/23371/diff/3/?file=626998#file626998line82>
> >
> >     don't think this needs to be volatile since it's no longer static.

Yeah, I had left all that because initially I made it so that doWork as called 
when live_nodes changed, and so could happen from multiple threads. That didn't 
turn out very nice though.


> On July 10, 2014, 1:42 a.m., Gregory Chanan wrote:
> > trunk/solr/core/src/test/org/apache/solr/cloud/SharedFSAutoReplicaFailoverTest.java,
> >  line 133
> > <https://reviews.apache.org/r/23371/diff/3/?file=627026#file627026line133>
> >
> >     Check collection2 as well?

For collection2, we are just ensuring that it *won't* use autoAddReplicas. So 
it won't pass this waitForAllActiveAndLive check.


> On July 10, 2014, 1:42 a.m., Gregory Chanan wrote:
> > trunk/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java, 
> > line 159
> > <https://reviews.apache.org/r/23371/diff/3/?file=627039#file627039line159>
> >
> >     I asked this on the JIRA, why is maxShardPerNode being checked but not 
> > replication factor? 
> > https://cwiki.apache.org/confluence/display/solr/Collections+API says 
> > replication factor is required, but not maxShards?
> >     
> >     Also, why does getMaxShardsPerNode return an int but 
> > getReplicationFactor an Integer?

We make create calls to match replication factor. This just makes sure we don't 
do that beyond maxShardPerNode.


> On July 10, 2014, 1:42 a.m., Gregory Chanan wrote:
> > trunk/solr/core/src/test/org/apache/solr/cloud/SharedFSAutoReplicaFailoverUtilsTest.java,
> >  line 84
> > <https://reviews.apache.org/r/23371/diff/3/?file=627027#file627027line84>
> >
> >     Made some comments in the jira about how I think this could be more 
> > understandable.

I've made some tweaks based on the feedback.


- Mark


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


On July 9, 2014, 5:36 p.m., Mark Miller wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23371/
> -----------------------------------------------------------
> 
> (Updated July 9, 2014, 5:36 p.m.)
> 
> 
> Review request for lucene.
> 
> 
> Bugs: SOLR-5656
>     https://issues.apache.org/jira/browse/SOLR-5656
> 
> 
> Repository: lucene
> 
> 
> Description
> -------
> 
> First svn patch for SOLR-5656: Add autoAddReplicas feature for shared file 
> systems.
> 
> 
> Diffs
> -----
> 
>   trunk/solr/cloud-dev/control.sh 1608973 
>   trunk/solr/cloud-dev/functions.sh 1608973 
>   trunk/solr/cloud-dev/solrcloud-start-existing.sh 1608973 
>   trunk/solr/cloud-dev/solrcloud-start.sh 1608973 
>   trunk/solr/cloud-dev/stop.sh 1608973 
>   
> trunk/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestSolrEntityProcessorEndToEnd.java
>  1608973 
>   trunk/solr/core/src/java/org/apache/solr/cloud/Assign.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/cloud/CloudUtil.java PRE-CREATION 
>   trunk/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/cloud/Overseer.java 1608973 
>   
> trunk/solr/core/src/java/org/apache/solr/cloud/OverseerAutoReplicaFailoverThread.java
>  PRE-CREATION 
>   
> trunk/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java
>  1608973 
>   trunk/solr/core/src/java/org/apache/solr/cloud/ZkController.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/core/ConfigSolr.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXmlOld.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/core/DirectoryFactory.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/core/HdfsDirectoryFactory.java 
> 1608973 
>   
> trunk/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
>  1608973 
>   
> trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java 
> 1608973 
>   trunk/solr/core/src/java/org/apache/solr/request/LocalSolrQueryRequest.java 
> 1608973 
>   trunk/solr/core/src/java/org/apache/solr/update/HdfsUpdateLog.java 1608973 
>   trunk/solr/core/src/java/org/apache/solr/update/UpdateShardHandler.java 
> 1608973 
>   trunk/solr/core/src/test-files/log4j.properties 1608973 
>   trunk/solr/core/src/test-files/solr/solr-no-core.xml 1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java 
> 1608973 
>   
> trunk/solr/core/src/test/org/apache/solr/cloud/ChaosMonkeyShardSplitTest.java 
> 1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/ClusterStateUpdateTest.java 
> 1608973 
>   
> trunk/solr/core/src/test/org/apache/solr/cloud/CollectionsAPIDistributedZkTest.java
>  1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/CustomCollectionTest.java 
> 1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java 
> 1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java 
> 1608973 
>   
> trunk/solr/core/src/test/org/apache/solr/cloud/OverseerCollectionProcessorTest.java
>  1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/OverseerRolesTest.java 
> 1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/OverseerTest.java 1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/ShardRoutingCustomTest.java 
> 1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/ShardSplitTest.java 1608973 
>   
> trunk/solr/core/src/test/org/apache/solr/cloud/SharedFSAutoReplicaFailoverTest.java
>  PRE-CREATION 
>   
> trunk/solr/core/src/test/org/apache/solr/cloud/SharedFSAutoReplicaFailoverUtilsTest.java
>  PRE-CREATION 
>   trunk/solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java 
> 1608973 
>   trunk/solr/core/src/test/org/apache/solr/cloud/hdfs/HdfsTestUtil.java 
> 1608973 
>   
> trunk/solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java 
> 1608973 
>   
> trunk/solr/core/src/test/org/apache/solr/handler/TestReplicationHandlerBackup.java
>  1608973 
>   trunk/solr/core/src/test/org/apache/solr/search/TestRecoveryHdfs.java 
> 1608973 
>   trunk/solr/core/src/test/org/apache/solr/util/MockConfigSolr.java 
> PRE-CREATION 
>   trunk/solr/example/solr/solr.xml 1608973 
>   
> trunk/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
>  1608973 
>   trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ClosableThread.java 
> 1608973 
>   trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java 
> 1608973 
>   
> trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ClusterStateUtil.java 
> PRE-CREATION 
>   trunk/solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java 
> 1608973 
>   trunk/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java 
> 1608973 
>   trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java 
> 1608973 
>   
> trunk/solr/solrj/src/test/org/apache/solr/client/solrj/SolrExampleTestBase.java
>  1608973 
>   
> trunk/solr/solrj/src/test/org/apache/solr/client/solrj/TestLBHttpSolrServer.java
>  1608973 
>   
> trunk/solr/solrj/src/test/org/apache/solr/client/solrj/embedded/JettyWebappTest.java
>  1608973 
>   
> trunk/solr/test-framework/src/java/org/apache/solr/BaseDistributedSearchTestCase.java
>  1608973 
>   trunk/solr/test-framework/src/java/org/apache/solr/SolrJettyTestBase.java 
> 1608973 
>   
> trunk/solr/test-framework/src/java/org/apache/solr/cloud/AbstractFullDistribZkTestBase.java
>  1608973 
>   trunk/solr/test-framework/src/java/org/apache/solr/cloud/ChaosMonkey.java 
> 1608973 
>   
> trunk/solr/test-framework/src/java/org/apache/solr/cloud/CloudInspectUtil.java
>  1608973 
>   
> trunk/solr/test-framework/src/java/org/apache/solr/cloud/MockSolrZkClient.java
>  PRE-CREATION 
>   
> trunk/solr/test-framework/src/java/org/apache/solr/cloud/MockZkStateReader.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/23371/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mark Miller
> 
>

Reply via email to