dsmiley commented on a change in pull request #2391:
URL: https://github.com/apache/lucene-solr/pull/2391#discussion_r590352385
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java
##########
@@ -300,6 +307,9 @@ public Integer getReplicationFactor() {
return replicationFactor;
}
+ /**
+ * Return non-null configName.
Review comment:
```suggestion
* Return non-null configset name.
```
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/DocCollection.java
##########
@@ -88,10 +88,17 @@ public DocCollection(String name, Map<String, Slice>
slices, Map<String, Object>
* @param zkVersion The version of the Collection node in Zookeeper (used
for conditional updates).
*/
public DocCollection(String name, Map<String, Slice> slices, Map<String,
Object> props, DocRouter router, int zkVersion) {
- super(props);
- if (props == null || props.containsKey("baseConfigSet")) {
+ if (props == null) {
Review comment:
null || props.isEmpty() can be combined
##########
File path: solr/core/src/java/org/apache/solr/cloud/CloudConfigSetService.java
##########
@@ -86,12 +86,11 @@ public SolrResourceLoader
createCoreResourceLoader(CoreDescriptor cd) {
// The configSet is read from ZK and populated. Ignore CD's pre-existing
configSet; only populated in standalone
final String configSetName;
- try {
- configSetName = zkController.getZkStateReader().readConfigName(colName);
- cd.setConfigSet(configSetName);
- } catch (KeeperException ex) {
- throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR,
"Trouble resolving configSet for collection " + colName + ": " +
ex.getMessage());
+ configSetName =
zkController.getZkStateReader().getClusterState().getCollection(colName).getConfigName();
+ if (configSetName == null) {
Review comment:
still an issue?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
##########
@@ -554,12 +554,11 @@ private void modifyCollection(ClusterState clusterState,
ZkNodeProps message, @S
final String collectionName =
message.getStr(ZkStateReader.COLLECTION_PROP);
//the rest of the processing is based on writing cluster state properties
//remove the property here to avoid any errors down the pipeline due to
this property appearing
- String configName = (String)
message.getProperties().remove(CollectionAdminParams.COLL_CONF);
+ String configName = (String)
message.getProperties().get(CollectionAdminParams.COLL_CONF);
Review comment:
just observing that this innocent looking change seems important to this
PR. Previously this data had disappeared from the state.
##########
File path:
solr/core/src/java/org/apache/solr/cloud/OverseerConfigSetMessageHandler.java
##########
@@ -367,12 +367,7 @@ private void deleteConfigSet(String configSetName, boolean
force) throws IOExcep
for (Map.Entry<String, DocCollection> entry :
zkStateReader.getClusterState().getCollectionsMap().entrySet()) {
String configName = null;
- try {
- configName = zkStateReader.readConfigName(entry.getKey());
- } catch (KeeperException ex) {
- throw new SolrException(ErrorCode.BAD_REQUEST,
- "Can not delete ConfigSet as it is currently being used by
collection [" + entry.getKey() + "]");
- }
+ configName = entry.getValue().getConfigName();
Review comment:
combine declaration and initialization
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
