madrob commented on a change in pull request #559: URL: https://github.com/apache/solr/pull/559#discussion_r791157908
########## File path: solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java ########## @@ -153,6 +156,21 @@ default Tracer getTracer() { default Span getSpan() { return NoopSpan.INSTANCE; } + + default CoreContainer getCoreContainer() { + return getCore() == null ? null : getCore().getCoreContainer(); + } + + default void runAsync(Runnable r) { + getCore().runAsync(r); Review comment: could return null? ########## File path: solr/core/src/java/org/apache/solr/cluster/placement/plugins/SimplePlacementFactory.java ########## @@ -99,7 +99,7 @@ public PlacementPlugin createPluginInstance() { private Map<Node, ReplicaCount> getNodeVsShardCount(PlacementContext placementContext) { HashMap<Node, ReplicaCount> nodeVsShardCount = new HashMap<>(); - for (Node s : placementContext.getCluster().getLiveDataNodes()) { + for (Node s : placementContext.getCluster().getLiveNodes()) { Review comment: Why? ########## File path: solr/core/src/java/org/apache/solr/search/stats/ExactStatsCache.java ########## @@ -230,10 +230,10 @@ protected void doSendGlobalStats(ResponseBuilder rb, ShardRequest outgoing) { Map<String,TermStats> globalTermStats = new HashMap<>(); Map<String,CollectionStats> globalColStats = new HashMap<>(); // aggregate collection stats, only for the field in terms - String collectionName = rb.req.getCore().getCoreDescriptor().getCollectionName(); - if (collectionName == null) { + String collectionName = rb.req.getCloudDescriptor().getCollectionName(); + /* if (collectionName == null) { collectionName = rb.req.getCore().getCoreDescriptor().getName(); - } + }*/ Review comment: Why can't this be null anymore? It was a strange check to begin with, makes me assume there was a subtle edge case here. ########## File path: solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java ########## @@ -255,9 +257,13 @@ protected HttpSolrCall getHttpSolrCall(HttpServletRequest request, HttpServletRe throw new SolrException(ErrorCode.SERVER_ERROR, "Core Container Unavailable"); } if (isV2Enabled && (path.startsWith("/____v2/") || path.equals("/____v2"))) { - return new V2HttpCall(this, cores, request, response, false); + return isCoordinator ? + new CoordinatorHttpSolrCall(this, cores, request, response, false) : + new V2HttpCall(this, cores, request, response, false); } else { - return new HttpSolrCall(this, cores, request, response, retry); + return isCoordinator ? + new CoordinatorHttpSolrCall(this, cores, request, response, retry) : + new HttpSolrCall(this, cores, request, response, retry); Review comment: This feels very fragile plugging into SDF this way. I thought part of the goal with node roles was that we make things more modular instead of coupled. ########## File path: solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java ########## @@ -83,11 +83,11 @@ static PlacementRequestImpl toPlacementRequest(Cluster cluster, SolrCollection s if (assignRequest.nodes != null) { nodes = SimpleClusterAbstractionsImpl.NodeImpl.getNodes(assignRequest.nodes); - for (Node n: nodes) { + /* for (Node n: nodes) { if (!cluster.getLiveDataNodes().contains(n)) { throw new Assign.AssignmentException("Bad assign request: specified node is a non-data hosting node (" + n.getName() + ") for collection " + solrCollection.getName()); } - } + }*/ Review comment: Why? ########## File path: solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java ########## @@ -0,0 +1,255 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.servlet; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.lang.invoke.MethodHandles; +import java.security.Principal; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.cloud.CloudDescriptor; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.ContentStream; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.CoreDescriptor; +import org.apache.solr.core.SolrCore; +import org.apache.solr.request.LocalSolrQueryRequest; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.schema.IndexSchema; +import org.apache.solr.search.SolrIndexSearcher; +import org.apache.solr.util.RTimerTree; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class CoordinatorHttpSolrCall extends HttpSolrCall { + public static final String SYNTHETIC_COLL_PREFIX = "SYNTHETIC-CONF-COLL_"; + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private String collectionName; + + public CoordinatorHttpSolrCall(SolrDispatchFilter solrDispatchFilter, CoreContainer cores, HttpServletRequest request, + HttpServletResponse response, boolean retry) { + super(solrDispatchFilter, cores, request, response, retry); + } + + @Override + protected SolrCore getCoreByCollection(String collectionName, boolean isPreferLeader) { + this.collectionName = collectionName; + SolrCore core = super.getCoreByCollection(collectionName, isPreferLeader); + if (core != null) return core; + return getCore(this, collectionName, isPreferLeader); + } + + @SuppressWarnings("unchecked") Review comment: Is this necessary? Please use proper generic types when appropriate. ########## File path: solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java ########## @@ -0,0 +1,255 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.servlet; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.lang.invoke.MethodHandles; +import java.security.Principal; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.cloud.CloudDescriptor; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.ContentStream; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.CoreDescriptor; +import org.apache.solr.core.SolrCore; +import org.apache.solr.request.LocalSolrQueryRequest; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.schema.IndexSchema; +import org.apache.solr.search.SolrIndexSearcher; +import org.apache.solr.util.RTimerTree; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class CoordinatorHttpSolrCall extends HttpSolrCall { + public static final String SYNTHETIC_COLL_PREFIX = "SYNTHETIC-CONF-COLL_"; + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private String collectionName; + + public CoordinatorHttpSolrCall(SolrDispatchFilter solrDispatchFilter, CoreContainer cores, HttpServletRequest request, + HttpServletResponse response, boolean retry) { + super(solrDispatchFilter, cores, request, response, retry); + } + + @Override + protected SolrCore getCoreByCollection(String collectionName, boolean isPreferLeader) { + this.collectionName = collectionName; + SolrCore core = super.getCoreByCollection(collectionName, isPreferLeader); + if (core != null) return core; + return getCore(this, collectionName, isPreferLeader); + } + + @SuppressWarnings("unchecked") + public static SolrCore getCore(HttpSolrCall solrCall, String collectionName, boolean isPreferLeader) { + Map<String, String> coreNameMapping= (Map<String, String>) solrCall.cores.getObjectCache().computeIfAbsent(CoordinatorHttpSolrCall.class.getName(), + s -> new ConcurrentHashMap<>()); + String sytheticCoreName = coreNameMapping.get(collectionName); + if (sytheticCoreName != null) { + return solrCall.cores.getCore(sytheticCoreName); + } else { + ZkStateReader zkStateReader = solrCall.cores.getZkController().getZkStateReader(); + ClusterState clusterState = zkStateReader.getClusterState(); + DocCollection coll = clusterState.getCollectionOrNull(collectionName, true); + if (coll != null) { + String confName = coll.getConfigName(); + String syntheticCollectionName = SYNTHETIC_COLL_PREFIX + confName; + DocCollection syntheticColl = clusterState.getCollectionOrNull(syntheticCollectionName); + if (syntheticColl == null) { + // no such collection. let's create one + log.info("synthetic collection: {} does not exist, creating.. ", syntheticCollectionName); + createColl(syntheticCollectionName, solrCall.cores, confName); + } + SolrCore core = solrCall.getCoreByCollection(syntheticCollectionName, isPreferLeader); + if (core != null) { + coreNameMapping.put(collectionName, core.getName()); + log.info("coordinator NODE , returns synthetic core " + core.getName()); + } else { + //this node does not have a replica. add one + log.info("this node does not have a replica of the synthetic collection: {} , adding replica ", syntheticCollectionName); + + addReplica(syntheticCollectionName, solrCall.cores); + core = solrCall.getCoreByCollection(syntheticCollectionName, isPreferLeader); + } + return core; + + } + return null; + } + + } + private static void addReplica(String syntheticCollectionName, CoreContainer cores) { + SolrQueryResponse rsp = new SolrQueryResponse(); + try { + cores.getCollectionsHandler().handleRequestBody(new LocalSolrQueryRequest(null, + CollectionAdminRequest.addReplicaToShard(syntheticCollectionName, "shard1") + .setCreateNodeSet(cores.getZkController().getNodeName()) + .getParams() + ), rsp); + if (rsp.getValues().get("success") == null) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Could not auto-create collection: " + Utils.toJSONString(rsp.getValues())); + } + } catch (SolrException e) { + throw e; + + } catch (Exception e) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); + } + + } + + private static void createColl(String syntheticCollectionName, CoreContainer cores, String confName) { + SolrQueryResponse rsp = new SolrQueryResponse(); + try { + SolrParams params = CollectionAdminRequest.createCollection(syntheticCollectionName, confName, 1, 1) + .setCreateNodeSet(cores.getZkController().getNodeName()).getParams(); + log.info("sending collection admin command : {}", Utils.toJSONString(params)); + cores.getCollectionsHandler() + .handleRequestBody(new LocalSolrQueryRequest(null, + params), rsp); + if (rsp.getValues().get("success") == null) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Could not create :" + syntheticCollectionName + " collection: " + Utils.toJSONString(rsp.getValues())); + } + } catch (SolrException e) { + throw e; + + } catch (Exception e) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e); + } + } + + @Override + protected void init() throws Exception { + super.init(); + if(action == SolrDispatchFilter.Action.PROCESS && core != null) { + solrReq = wrappedReq(solrReq, collectionName, this); + } + } + + public static SolrQueryRequest wrappedReq(SolrQueryRequest delegate, String collectionName, HttpSolrCall httpSolrCall) { + Properties p = new Properties(); + p.put(CoreDescriptor.CORE_COLLECTION, collectionName); + p.put(CloudDescriptor.REPLICA_TYPE, Replica.Type.PULL.toString()); + p.put(CoreDescriptor.CORE_SHARD, "_"); + + CloudDescriptor cloudDescriptor = new CloudDescriptor(delegate.getCore().getCoreDescriptor(), + delegate.getCore().getName(), p); + return new SolrQueryRequest() { Review comment: Can we create a new top level DelegateSolrQueryRequest that delegates all of its methods and then this overrides just the ones that change? Would be easier to understand I think. ########## File path: solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java ########## @@ -0,0 +1,255 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.servlet; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.lang.invoke.MethodHandles; +import java.security.Principal; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.cloud.CloudDescriptor; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.ContentStream; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.CoreDescriptor; +import org.apache.solr.core.SolrCore; +import org.apache.solr.request.LocalSolrQueryRequest; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.schema.IndexSchema; +import org.apache.solr.search.SolrIndexSearcher; +import org.apache.solr.util.RTimerTree; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class CoordinatorHttpSolrCall extends HttpSolrCall { Review comment: Javadoc on the implementation of synthetic collections and how they work would be helpful here. ########## File path: solr/core/src/java/org/apache/solr/api/CoordinatorV2HttpSolrCall.java ########## @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.api; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.SolrCore; +import org.apache.solr.servlet.CoordinatorHttpSolrCall; +import org.apache.solr.servlet.SolrDispatchFilter; + +public class CoordinatorV2HttpSolrCall extends V2HttpCall { Review comment: javadoc please ########## File path: solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java ########## @@ -255,9 +257,13 @@ protected HttpSolrCall getHttpSolrCall(HttpServletRequest request, HttpServletRe throw new SolrException(ErrorCode.SERVER_ERROR, "Core Container Unavailable"); } if (isV2Enabled && (path.startsWith("/____v2/") || path.equals("/____v2"))) { - return new V2HttpCall(this, cores, request, response, false); + return isCoordinator ? + new CoordinatorHttpSolrCall(this, cores, request, response, false) : Review comment: I assume this is supposed to be CoordinatorV2HttpSolrCall instead ########## File path: solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.search; + +import java.util.Collection; +import java.util.Collections; + +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.embedded.JettySolrRunner; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.NavigableObject; +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.NodeRoles; +import org.apache.solr.servlet.CoordinatorHttpSolrCall; +import org.junit.BeforeClass; + +public class TestCoordinatorRole extends SolrCloudTestCase { + + @BeforeClass + public static void setupCluster() throws Exception { + configureCluster(4) + .addConfig("conf", configset("cloud-minimal")) + .configure(); + } + + public void testSimple() throws Exception { + CloudSolrClient client = cluster.getSolrClient(); + String COLLECTION_NAME = "test_coll"; + String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX +"conf"; + CollectionAdminRequest + .createCollection(COLLECTION_NAME, "conf", 2, 2) + .process(cluster.getSolrClient()); + cluster.waitForActiveCollection(COLLECTION_NAME, 2, 4); + UpdateRequest ur = new UpdateRequest(); + for (int i = 0; i < 10; i++) { + SolrInputDocument doc2 = new SolrInputDocument(); + doc2.addField("id", "" + i); + doc2.addField("fld1_s", "1 value 1 value 1 value 1 value 1 value 1 value 1 value "); + doc2.addField("fld2_s", "2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value "); + doc2.addField("fld3_s", "3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value "); + doc2.addField("fld4_s", "4 value 4 value 4 value 4 value 4 value 4 value 4 value 4 value 4 value "); + doc2.addField("fld5_s", "5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value "); + ur.add(doc2); + } + + ur.commit(client, COLLECTION_NAME); + QueryResponse rsp = client.query(COLLECTION_NAME, new SolrQuery("*:*")); + assertEquals(10, rsp.getResults().getNumFound()); + + System.setProperty(NodeRoles.NODE_ROLES_PROP, "coordinator:on"); + JettySolrRunner coordinatorJetty = null; + try { + coordinatorJetty = cluster.startJettySolrRunner(); + } finally { + System.clearProperty(NodeRoles.NODE_ROLES_PROP); + } Review comment: I really don't like this way of starting roles with certain properties and complained about it in the original node roles PR. But that was ignored, so I'll bring it up again here. We need a programmatic way to do it, rather than just system props. ########## File path: solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java ########## @@ -0,0 +1,255 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.servlet; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import java.lang.invoke.MethodHandles; +import java.security.Principal; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.ConcurrentHashMap; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.cloud.CloudDescriptor; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.ContentStream; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.CoreDescriptor; +import org.apache.solr.core.SolrCore; +import org.apache.solr.request.LocalSolrQueryRequest; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.schema.IndexSchema; +import org.apache.solr.search.SolrIndexSearcher; +import org.apache.solr.util.RTimerTree; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class CoordinatorHttpSolrCall extends HttpSolrCall { + public static final String SYNTHETIC_COLL_PREFIX = "SYNTHETIC-CONF-COLL_"; + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private String collectionName; + + public CoordinatorHttpSolrCall(SolrDispatchFilter solrDispatchFilter, CoreContainer cores, HttpServletRequest request, + HttpServletResponse response, boolean retry) { + super(solrDispatchFilter, cores, request, response, retry); + } + + @Override + protected SolrCore getCoreByCollection(String collectionName, boolean isPreferLeader) { + this.collectionName = collectionName; + SolrCore core = super.getCoreByCollection(collectionName, isPreferLeader); + if (core != null) return core; + return getCore(this, collectionName, isPreferLeader); + } + + @SuppressWarnings("unchecked") + public static SolrCore getCore(HttpSolrCall solrCall, String collectionName, boolean isPreferLeader) { + Map<String, String> coreNameMapping= (Map<String, String>) solrCall.cores.getObjectCache().computeIfAbsent(CoordinatorHttpSolrCall.class.getName(), + s -> new ConcurrentHashMap<>()); + String sytheticCoreName = coreNameMapping.get(collectionName); + if (sytheticCoreName != null) { + return solrCall.cores.getCore(sytheticCoreName); + } else { Review comment: There can be race conditions here? Multiple requests calculating the syntheticCoreName ########## File path: solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.search; + +import java.util.Collection; +import java.util.Collections; + +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.embedded.JettySolrRunner; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.NavigableObject; +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.NodeRoles; +import org.apache.solr.servlet.CoordinatorHttpSolrCall; +import org.junit.BeforeClass; + +public class TestCoordinatorRole extends SolrCloudTestCase { + + @BeforeClass + public static void setupCluster() throws Exception { + configureCluster(4) + .addConfig("conf", configset("cloud-minimal")) + .configure(); + } + + public void testSimple() throws Exception { + CloudSolrClient client = cluster.getSolrClient(); + String COLLECTION_NAME = "test_coll"; + String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX +"conf"; + CollectionAdminRequest + .createCollection(COLLECTION_NAME, "conf", 2, 2) + .process(cluster.getSolrClient()); + cluster.waitForActiveCollection(COLLECTION_NAME, 2, 4); + UpdateRequest ur = new UpdateRequest(); + for (int i = 0; i < 10; i++) { + SolrInputDocument doc2 = new SolrInputDocument(); + doc2.addField("id", "" + i); + doc2.addField("fld1_s", "1 value 1 value 1 value 1 value 1 value 1 value 1 value "); + doc2.addField("fld2_s", "2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value "); + doc2.addField("fld3_s", "3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value "); + doc2.addField("fld4_s", "4 value 4 value 4 value 4 value 4 value 4 value 4 value 4 value 4 value "); + doc2.addField("fld5_s", "5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value "); + ur.add(doc2); + } + + ur.commit(client, COLLECTION_NAME); + QueryResponse rsp = client.query(COLLECTION_NAME, new SolrQuery("*:*")); + assertEquals(10, rsp.getResults().getNumFound()); + + System.setProperty(NodeRoles.NODE_ROLES_PROP, "coordinator:on"); + JettySolrRunner coordinatorJetty = null; + try { + coordinatorJetty = cluster.startJettySolrRunner(); + } finally { + System.clearProperty(NodeRoles.NODE_ROLES_PROP); + } + NavigableObject result = (NavigableObject) Utils.executeGET(cluster.getSolrClient().getHttpClient(), Review comment: I don't understand how this is different from what happens today without all of your other changes. The receiving node would still forward the request, wouldn't it? Can you provide a test case or configuration where we could observe some kind of failure? ########## File path: solr/core/src/java/org/apache/solr/request/SolrQueryRequest.java ########## @@ -153,6 +156,21 @@ default Tracer getTracer() { default Span getSpan() { return NoopSpan.INSTANCE; } + + default CoreContainer getCoreContainer() { + return getCore() == null ? null : getCore().getCoreContainer(); + } + + default void runAsync(Runnable r) { + getCore().runAsync(r); + } + + default CloudDescriptor getCloudDescriptor() { + SolrCore core = getCore(); + if (core == null) return null; + return core.getCoreDescriptor().getCloudDescriptor(); + } + Review comment: Please add javadoc to all of these methods. ########## File path: solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java ########## @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.search; + +import java.util.Collection; +import java.util.Collections; + +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.embedded.JettySolrRunner; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.NavigableObject; +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.NodeRoles; +import org.apache.solr.servlet.CoordinatorHttpSolrCall; +import org.junit.BeforeClass; + +public class TestCoordinatorRole extends SolrCloudTestCase { + + @BeforeClass + public static void setupCluster() throws Exception { + configureCluster(4) + .addConfig("conf", configset("cloud-minimal")) + .configure(); + } + + public void testSimple() throws Exception { + CloudSolrClient client = cluster.getSolrClient(); + String COLLECTION_NAME = "test_coll"; + String SYNTHETIC_COLLECTION = CoordinatorHttpSolrCall.SYNTHETIC_COLL_PREFIX +"conf"; + CollectionAdminRequest + .createCollection(COLLECTION_NAME, "conf", 2, 2) + .process(cluster.getSolrClient()); + cluster.waitForActiveCollection(COLLECTION_NAME, 2, 4); + UpdateRequest ur = new UpdateRequest(); + for (int i = 0; i < 10; i++) { + SolrInputDocument doc2 = new SolrInputDocument(); + doc2.addField("id", "" + i); + doc2.addField("fld1_s", "1 value 1 value 1 value 1 value 1 value 1 value 1 value "); + doc2.addField("fld2_s", "2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value 2 value "); + doc2.addField("fld3_s", "3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value 3 value "); + doc2.addField("fld4_s", "4 value 4 value 4 value 4 value 4 value 4 value 4 value 4 value 4 value "); + doc2.addField("fld5_s", "5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value 5 value "); Review comment: Are these used by the test at all? -- 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. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org