[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r244007693
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -42,15 +53,36 @@ public
DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
}
@Override
- String getCollectionName(CloudDescriptor cloudDesc) {
+ String computeCollectionName(CloudDescriptor cloudDesc) {
return null;
}
@Override
- Replica.Type getReplicaType(CloudDescriptor cloudDesc) {
+ Replica.Type computeReplicaType(CloudDescriptor cloudDesc) {
return Replica.Type.NRT;
}
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
--- End diff --
Hopefully I addressed your concerns with latest commit.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243999855
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.update.processor;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.Future;
+
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.CommitUpdateCommand;
+import org.apache.solr.update.DeleteUpdateCommand;
+import org.apache.solr.update.UpdateCommand;
+import org.apache.solr.util.TestInjection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DistributedStandaloneUpdateProcessor extends
DistributedUpdateProcessor {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
UpdateRequestProcessor next) {
+super(req, rsp, next);
+ }
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
AtomicUpdateDocumentMerger docMerger,
+ UpdateRequestProcessor next)
{
+super(req, rsp, docMerger, next);
+ }
+
+ @Override
+ Replica.Type computeReplicaType() {
+return Replica.Type.NRT;
+ }
+
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
+Set> pending = new HashSet<>();
+if (replicaType == Replica.Type.TLOG) {
+ if (isLeader) {
+super.processCommit(cmd);
+ }
+} else if (replicaType == Replica.Type.PULL) {
+ log.warn("Commit not supported on replicas of type " +
Replica.Type.PULL);
+} else {
+ // NRT replicas will always commit
+ super.processCommit(cmd);
+}
+ }
+
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+isLeader = getNonZkLeaderAssumption(req);
+
+super.processAdd(cmd);
+ }
+
+ @Override
+ protected void doDeleteById(DeleteUpdateCommand cmd) throws IOException {
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteById(cmd);
+ }
+
+ @Override
+ protected void doDeleteByQuery(DeleteUpdateCommand cmd) throws
IOException {
+// even in non zk mode, tests simulate updates from a leader
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteByQuery(cmd, null, null);
+ }
+
+ @Override
+ public void finish() throws IOException {
--- End diff --
nevermind; just leave this be then. Lets keep our changes to more
clear/obvious ones.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243995067
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.update.processor;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.Future;
+
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.CommitUpdateCommand;
+import org.apache.solr.update.DeleteUpdateCommand;
+import org.apache.solr.update.UpdateCommand;
+import org.apache.solr.util.TestInjection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DistributedStandaloneUpdateProcessor extends
DistributedUpdateProcessor {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
UpdateRequestProcessor next) {
+super(req, rsp, next);
+ }
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
AtomicUpdateDocumentMerger docMerger,
+ UpdateRequestProcessor next)
{
+super(req, rsp, docMerger, next);
+ }
+
+ @Override
+ Replica.Type computeReplicaType() {
+return Replica.Type.NRT;
+ }
+
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
+Set> pending = new HashSet<>();
+if (replicaType == Replica.Type.TLOG) {
+ if (isLeader) {
+super.processCommit(cmd);
+ }
+} else if (replicaType == Replica.Type.PULL) {
+ log.warn("Commit not supported on replicas of type " +
Replica.Type.PULL);
+} else {
+ // NRT replicas will always commit
+ super.processCommit(cmd);
+}
+ }
+
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+isLeader = getNonZkLeaderAssumption(req);
+
+super.processAdd(cmd);
+ }
+
+ @Override
+ protected void doDeleteById(DeleteUpdateCommand cmd) throws IOException {
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteById(cmd);
+ }
+
+ @Override
+ protected void doDeleteByQuery(DeleteUpdateCommand cmd) throws
IOException {
+// even in non zk mode, tests simulate updates from a leader
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteByQuery(cmd, null, null);
+ }
+
+ @Override
+ public void finish() throws IOException {
--- End diff --
I'm still trying to dig in further and see why the last line (`if (next !=
null && nodes == null) next.finish();`) was put in an if and not an else if
statement.
nodes is supposed to be the replicas of the current nodes, and when
zkEnabled was false it was set to null. I am still no sure why this was in a
completely separate if statement.
@dsmiley , WDYT?
---
-
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243850805
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.update.processor;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.Future;
+
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.CommitUpdateCommand;
+import org.apache.solr.update.DeleteUpdateCommand;
+import org.apache.solr.update.UpdateCommand;
+import org.apache.solr.util.TestInjection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DistributedStandaloneUpdateProcessor extends
DistributedUpdateProcessor {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
UpdateRequestProcessor next) {
+super(req, rsp, next);
+ }
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
AtomicUpdateDocumentMerger docMerger,
+ UpdateRequestProcessor next)
{
+super(req, rsp, docMerger, next);
+ }
+
+ @Override
+ Replica.Type computeReplicaType() {
+return Replica.Type.NRT;
+ }
+
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
+Set> pending = new HashSet<>();
+if (replicaType == Replica.Type.TLOG) {
+ if (isLeader) {
+super.processCommit(cmd);
+ }
+} else if (replicaType == Replica.Type.PULL) {
+ log.warn("Commit not supported on replicas of type " +
Replica.Type.PULL);
+} else {
+ // NRT replicas will always commit
+ super.processCommit(cmd);
+}
+ }
+
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+isLeader = getNonZkLeaderAssumption(req);
+
+super.processAdd(cmd);
+ }
+
+ @Override
+ protected void doDeleteById(DeleteUpdateCommand cmd) throws IOException {
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteById(cmd);
+ }
+
+ @Override
+ protected void doDeleteByQuery(DeleteUpdateCommand cmd) throws
IOException {
+// even in non zk mode, tests simulate updates from a leader
+isLeader = getNonZkLeaderAssumption(req);
--- End diff --
I think it can also be used in the zkProcessor,
I'll investigate further.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243850226
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.update.processor;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.Future;
+
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.CommitUpdateCommand;
+import org.apache.solr.update.DeleteUpdateCommand;
+import org.apache.solr.update.UpdateCommand;
+import org.apache.solr.util.TestInjection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DistributedStandaloneUpdateProcessor extends
DistributedUpdateProcessor {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
UpdateRequestProcessor next) {
+super(req, rsp, next);
+ }
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
AtomicUpdateDocumentMerger docMerger,
+ UpdateRequestProcessor next)
{
+super(req, rsp, docMerger, next);
+ }
+
+ @Override
+ Replica.Type computeReplicaType() {
+return Replica.Type.NRT;
+ }
+
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
+Set> pending = new HashSet<>();
+if (replicaType == Replica.Type.TLOG) {
+ if (isLeader) {
+super.processCommit(cmd);
+ }
+} else if (replicaType == Replica.Type.PULL) {
+ log.warn("Commit not supported on replicas of type " +
Replica.Type.PULL);
+} else {
+ // NRT replicas will always commit
+ super.processCommit(cmd);
+}
+ }
+
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+isLeader = getNonZkLeaderAssumption(req);
+
+super.processAdd(cmd);
+ }
+
+ @Override
+ protected void doDeleteById(DeleteUpdateCommand cmd) throws IOException {
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteById(cmd);
+ }
+
+ @Override
+ protected void doDeleteByQuery(DeleteUpdateCommand cmd) throws
IOException {
+// even in non zk mode, tests simulate updates from a leader
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteByQuery(cmd, null, null);
+ }
+
+ @Override
+ public void finish() throws IOException {
--- End diff --
Well, previously, the code was:
```
@Override
public void finish() throws IOException {
assert ! finished : "lifecycle sanity check";
finished = true;
if (zkEnabled) doFinish();
if (next != null && nodes == null) next.finish();
}
```
So nodes was always set to null when zkEnabled was false(see
DistributedUpdateProcessor#setupRequest on master).
So, I just split
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243847609
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
---
@@ -1250,23 +1250,10 @@ private UpdateCommand
fetchFullUpdateFromLeader(AddUpdateCommand inplaceAdd, lon
params.set("onlyIfActive", true);
SolrRequest ur = new
GenericSolrRequest(METHOD.GET, "/get", params);
-String leaderUrl = req.getParams().get(DISTRIB_FROM);
-
-if (leaderUrl == null) {
- // An update we're dependent upon didn't arrive! This is unexpected.
Perhaps likely our leader is
- // down or partitioned from us for some reason. Lets force refresh
cluster state, and request the
- // leader for the update.
- if (zkController == null) { // we should be in cloud mode, but wtf?
could be a unit test
-throw new SolrException(ErrorCode.SERVER_ERROR, "Can't find
document with id=" + id + ", but fetching from leader "
-+ "failed since we're not in cloud mode.");
- }
- Replica leader;
- try {
-leader =
zkController.getZkStateReader().getLeaderRetry(collection,
cloudDesc.getShardId());
- } catch (InterruptedException e) {
-throw new SolrException(ErrorCode.SERVER_ERROR, "Exception during
fetching from leader.", e);
- }
- leaderUrl = leader.getCoreUrl();
+String leaderUrl = getLeaderUrl(id);
--- End diff --
I looked again and it seems fine. I could be the diff I was looking at
confused the matter.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243847326
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.update.processor;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.Future;
+
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.CommitUpdateCommand;
+import org.apache.solr.update.DeleteUpdateCommand;
+import org.apache.solr.update.UpdateCommand;
+import org.apache.solr.util.TestInjection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DistributedStandaloneUpdateProcessor extends
DistributedUpdateProcessor {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
UpdateRequestProcessor next) {
+super(req, rsp, next);
+ }
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
AtomicUpdateDocumentMerger docMerger,
+ UpdateRequestProcessor next)
{
+super(req, rsp, docMerger, next);
+ }
+
+ @Override
+ Replica.Type computeReplicaType() {
+return Replica.Type.NRT;
+ }
+
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
+Set> pending = new HashSet<>();
+if (replicaType == Replica.Type.TLOG) {
+ if (isLeader) {
+super.processCommit(cmd);
+ }
+} else if (replicaType == Replica.Type.PULL) {
+ log.warn("Commit not supported on replicas of type " +
Replica.Type.PULL);
+} else {
+ // NRT replicas will always commit
+ super.processCommit(cmd);
+}
+ }
+
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+isLeader = getNonZkLeaderAssumption(req);
+
+super.processAdd(cmd);
+ }
+
+ @Override
+ protected void doDeleteById(DeleteUpdateCommand cmd) throws IOException {
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteById(cmd);
+ }
+
+ @Override
+ protected void doDeleteByQuery(DeleteUpdateCommand cmd) throws
IOException {
+// even in non zk mode, tests simulate updates from a leader
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteByQuery(cmd, null, null);
+ }
+
+ @Override
+ public void finish() throws IOException {
--- End diff --
isn't this what a base finish() will do? so why subclass. If there really
is a reason then a comment here would help
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243847270
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -0,0 +1,121 @@
+/*
+ * 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.update.processor;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.Future;
+
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.CommitUpdateCommand;
+import org.apache.solr.update.DeleteUpdateCommand;
+import org.apache.solr.update.UpdateCommand;
+import org.apache.solr.util.TestInjection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DistributedStandaloneUpdateProcessor extends
DistributedUpdateProcessor {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
UpdateRequestProcessor next) {
+super(req, rsp, next);
+ }
+
+ public DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
+ SolrQueryResponse rsp,
AtomicUpdateDocumentMerger docMerger,
+ UpdateRequestProcessor next)
{
+super(req, rsp, docMerger, next);
+ }
+
+ @Override
+ Replica.Type computeReplicaType() {
+return Replica.Type.NRT;
+ }
+
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
+Set> pending = new HashSet<>();
+if (replicaType == Replica.Type.TLOG) {
+ if (isLeader) {
+super.processCommit(cmd);
+ }
+} else if (replicaType == Replica.Type.PULL) {
+ log.warn("Commit not supported on replicas of type " +
Replica.Type.PULL);
+} else {
+ // NRT replicas will always commit
+ super.processCommit(cmd);
+}
+ }
+
+ @Override
+ public void processAdd(AddUpdateCommand cmd) throws IOException {
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+isLeader = getNonZkLeaderAssumption(req);
+
+super.processAdd(cmd);
+ }
+
+ @Override
+ protected void doDeleteById(DeleteUpdateCommand cmd) throws IOException {
+isLeader = getNonZkLeaderAssumption(req);
+super.doDeleteById(cmd);
+ }
+
+ @Override
+ protected void doDeleteByQuery(DeleteUpdateCommand cmd) throws
IOException {
+// even in non zk mode, tests simulate updates from a leader
+isLeader = getNonZkLeaderAssumption(req);
--- End diff --
I think just call setupRequest instead of getNonZkLeaderAssumption?
setupRequest calls that and sets isLeader. This comment goes for all/most of
the methods here.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243844969
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -42,15 +53,36 @@ public
DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
}
@Override
- String getCollectionName(CloudDescriptor cloudDesc) {
+ String computeCollectionName(CloudDescriptor cloudDesc) {
return null;
}
@Override
- Replica.Type getReplicaType(CloudDescriptor cloudDesc) {
+ Replica.Type computeReplicaType(CloudDescriptor cloudDesc) {
return Replica.Type.NRT;
}
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
--- End diff --
I guess I missed the fact that replicas in standalone mode are always NRT,
right?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243844755
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
---
@@ -1250,23 +1250,10 @@ private UpdateCommand
fetchFullUpdateFromLeader(AddUpdateCommand inplaceAdd, lon
params.set("onlyIfActive", true);
SolrRequest ur = new
GenericSolrRequest(METHOD.GET, "/get", params);
-String leaderUrl = req.getParams().get(DISTRIB_FROM);
-
-if (leaderUrl == null) {
- // An update we're dependent upon didn't arrive! This is unexpected.
Perhaps likely our leader is
- // down or partitioned from us for some reason. Lets force refresh
cluster state, and request the
- // leader for the update.
- if (zkController == null) { // we should be in cloud mode, but wtf?
could be a unit test
-throw new SolrException(ErrorCode.SERVER_ERROR, "Can't find
document with id=" + id + ", but fetching from leader "
-+ "failed since we're not in cloud mode.");
- }
- Replica leader;
- try {
-leader =
zkController.getZkStateReader().getLeaderRetry(collection,
cloudDesc.getShardId());
- } catch (InterruptedException e) {
-throw new SolrException(ErrorCode.SERVER_ERROR, "Exception during
fetching from leader.", e);
- }
- leaderUrl = leader.getCoreUrl();
+String leaderUrl = getLeaderUrl(id);
--- End diff --
Hmmm, if it is crucial to keep the exception message identical,
I could throw the exceptions inside getLeaderUrl(the abstract method).
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243844185
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
---
@@ -1250,23 +1250,10 @@ private UpdateCommand
fetchFullUpdateFromLeader(AddUpdateCommand inplaceAdd, lon
params.set("onlyIfActive", true);
SolrRequest ur = new
GenericSolrRequest(METHOD.GET, "/get", params);
-String leaderUrl = req.getParams().get(DISTRIB_FROM);
-
-if (leaderUrl == null) {
- // An update we're dependent upon didn't arrive! This is unexpected.
Perhaps likely our leader is
- // down or partitioned from us for some reason. Lets force refresh
cluster state, and request the
- // leader for the update.
- if (zkController == null) { // we should be in cloud mode, but wtf?
could be a unit test
-throw new SolrException(ErrorCode.SERVER_ERROR, "Can't find
document with id=" + id + ", but fetching from leader "
-+ "failed since we're not in cloud mode.");
- }
- Replica leader;
- try {
-leader =
zkController.getZkStateReader().getLeaderRetry(collection,
cloudDesc.getShardId());
- } catch (InterruptedException e) {
-throw new SolrException(ErrorCode.SERVER_ERROR, "Exception during
fetching from leader.", e);
- }
- leaderUrl = leader.getCoreUrl();
+String leaderUrl = getLeaderUrl(id);
--- End diff --
I'm not sure how this diff is a refactoring (keeps semantics identical).
For example... the exception message isn't the same.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243843852
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -42,15 +53,36 @@ public
DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
}
@Override
- String getCollectionName(CloudDescriptor cloudDesc) {
+ String computeCollectionName(CloudDescriptor cloudDesc) {
return null;
}
@Override
- Replica.Type getReplicaType(CloudDescriptor cloudDesc) {
+ Replica.Type computeReplicaType(CloudDescriptor cloudDesc) {
return Replica.Type.NRT;
}
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
--- End diff --
I didn't see any use for the completionService in the previous processor. I
meant to ask whether there might be a particular reason it is needed, since I
didn't see any uses for this service.
Should I remove it from both zk and standalone processors, or just this one?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243843542
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -42,15 +53,36 @@ public
DistributedStandaloneUpdateProcessor(SolrQueryRequest req,
}
@Override
- String getCollectionName(CloudDescriptor cloudDesc) {
+ String computeCollectionName(CloudDescriptor cloudDesc) {
return null;
}
@Override
- Replica.Type getReplicaType(CloudDescriptor cloudDesc) {
+ Replica.Type computeReplicaType(CloudDescriptor cloudDesc) {
return Replica.Type.NRT;
}
+ @Override
+ public void processCommit(CommitUpdateCommand cmd) throws IOException {
+
+assert TestInjection.injectFailUpdateRequests();
+
+updateCommand = cmd;
+
+CompletionService completionService = new
ExecutorCompletionService<>(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
--- End diff --
Why is all this stuff here in standalone mode?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r243628041
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java
---
@@ -425,4 +448,10 @@ void setupRequest(UpdateCommand cmd) {
nodes = setupRequest(dcmd.getId(), null);
}
}
+
+ @Override
+ protected boolean shouldCloneCmdDoc() {
+boolean willDistrib = isLeader && nodes != null && nodes.size() > 0;
--- End diff --
This could probably optimized, since cloneRequiredOnLeader is already set
during construction of the instance:
`if(!cloneRequiredOnLeader) {
return false;
}
// will distrib command
return isLeader && nodes != null && nodes.size() > 0;`
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241967695
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -56,8 +59,15 @@ public void doDeleteByQuery(DeleteUpdateCommand cmd)
throws IOException {
super.doDeleteByQuery(cmd, Collections.emptyList(), null);
}
+ @Override
+ protected void postProcessDeleteByQuery(DeleteUpdateCommand cmd,
+ List
replicas, DocCollection coll) throws IOException {
+// no op
+ }
+
@Override
boolean isLeader(UpdateCommand cmd) {
+updateCommand = cmd;
--- End diff --
Perhaps we should have another isLeader method that returns a boolean,
since the current method is used by other classes in the package to setup the
request and determine whether the core is the leader for the specified update
command.
I'll work on a patch ASAP.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241967674
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
---
@@ -1417,45 +1407,7 @@ private void doDeleteById(DeleteUpdateCommand cmd)
throws IOException {
return;
}
-if (zkEnabled && isLeader && !isSubShardLeader) {
- DocCollection coll =
zkController.getClusterState().getCollection(collection);
- List subShardLeaders = getSubShardLeaders(coll,
cloudDesc.getShardId(), cmd.getId(), null);
- // the list will actually have only one element for an add
request
- if (subShardLeaders != null && !subShardLeaders.isEmpty()) {
-ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
-params.set(DISTRIB_UPDATE_PARAM,
DistribPhase.FROMLEADER.toString());
-params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
-zkController.getBaseUrl(), req.getCore().getName()));
-params.set(DISTRIB_FROM_PARENT, cloudDesc.getShardId());
-cmdDistrib.distribDelete(cmd, subShardLeaders, params, true, null,
null);
- }
-
- final List nodesByRoutingRules =
getNodesByRoutingRules(zkController.getClusterState(), coll, cmd.getId(), null);
- if (nodesByRoutingRules != null && !nodesByRoutingRules.isEmpty()) {
-ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
-params.set(DISTRIB_UPDATE_PARAM,
DistribPhase.FROMLEADER.toString());
-params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
-zkController.getBaseUrl(), req.getCore().getName()));
-params.set(DISTRIB_FROM_COLLECTION, collection);
-params.set(DISTRIB_FROM_SHARD, cloudDesc.getShardId());
-cmdDistrib.distribDelete(cmd, nodesByRoutingRules, params, true,
null, null);
- }
-}
-
-if (nodes != null) {
- ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
- params.set(DISTRIB_UPDATE_PARAM,
- (isLeader || isSubShardLeader ?
DistribPhase.FROMLEADER.toString()
- : DistribPhase.TOLEADER.toString()));
- params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
- zkController.getBaseUrl(), req.getCore().getName()));
-
- if (req.getParams().get(UpdateRequest.MIN_REPFACT) != null) {
-// TODO: Kept for rolling upgrades only. Remove in Solr 9
-params.add(UpdateRequest.MIN_REPFACT,
req.getParams().get(UpdateRequest.MIN_REPFACT));
- }
- cmdDistrib.distribDelete(cmd, nodes, params, false,
rollupReplicationTracker, leaderReplicationTracker);
-}
+postProcessDeleteById(cmd);
--- End diff --
I can understand why you took the current approach. Lets put this off
towards near the end to see if it's straight-forward or not. Maybe it won't be
and we shouldn't do it.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241967637
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -56,8 +59,15 @@ public void doDeleteByQuery(DeleteUpdateCommand cmd)
throws IOException {
super.doDeleteByQuery(cmd, Collections.emptyList(), null);
}
+ @Override
+ protected void postProcessDeleteByQuery(DeleteUpdateCommand cmd,
+ List
replicas, DocCollection coll) throws IOException {
+// no op
+ }
+
@Override
boolean isLeader(UpdateCommand cmd) {
+updateCommand = cmd;
--- End diff --
+1 to setupRequest name. I don't think it needs to return "isLeader"; it
should set this and return void.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241954714
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -56,8 +59,15 @@ public void doDeleteByQuery(DeleteUpdateCommand cmd)
throws IOException {
super.doDeleteByQuery(cmd, Collections.emptyList(), null);
}
+ @Override
+ protected void postProcessDeleteByQuery(DeleteUpdateCommand cmd,
+ List
replicas, DocCollection coll) throws IOException {
+// no op
+ }
+
@Override
boolean isLeader(UpdateCommand cmd) {
+updateCommand = cmd;
--- End diff --
since this is a package private method, perhaps it should be renamed to
setupRequest(like the protected methods) just that this once takes an update
command( Distributed#setupRequest(UpdateCommand cmd) ). It should be stated in
the java doc that the method returns whether this node is the leader for the
supplied update command(cmd).
WDYT?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user barrotsteindev commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241949194
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
---
@@ -1417,45 +1407,7 @@ private void doDeleteById(DeleteUpdateCommand cmd)
throws IOException {
return;
}
-if (zkEnabled && isLeader && !isSubShardLeader) {
- DocCollection coll =
zkController.getClusterState().getCollection(collection);
- List subShardLeaders = getSubShardLeaders(coll,
cloudDesc.getShardId(), cmd.getId(), null);
- // the list will actually have only one element for an add
request
- if (subShardLeaders != null && !subShardLeaders.isEmpty()) {
-ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
-params.set(DISTRIB_UPDATE_PARAM,
DistribPhase.FROMLEADER.toString());
-params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
-zkController.getBaseUrl(), req.getCore().getName()));
-params.set(DISTRIB_FROM_PARENT, cloudDesc.getShardId());
-cmdDistrib.distribDelete(cmd, subShardLeaders, params, true, null,
null);
- }
-
- final List nodesByRoutingRules =
getNodesByRoutingRules(zkController.getClusterState(), coll, cmd.getId(), null);
- if (nodesByRoutingRules != null && !nodesByRoutingRules.isEmpty()) {
-ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
-params.set(DISTRIB_UPDATE_PARAM,
DistribPhase.FROMLEADER.toString());
-params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
-zkController.getBaseUrl(), req.getCore().getName()));
-params.set(DISTRIB_FROM_COLLECTION, collection);
-params.set(DISTRIB_FROM_SHARD, cloudDesc.getShardId());
-cmdDistrib.distribDelete(cmd, nodesByRoutingRules, params, true,
null, null);
- }
-}
-
-if (nodes != null) {
- ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
- params.set(DISTRIB_UPDATE_PARAM,
- (isLeader || isSubShardLeader ?
DistribPhase.FROMLEADER.toString()
- : DistribPhase.TOLEADER.toString()));
- params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
- zkController.getBaseUrl(), req.getCore().getName()));
-
- if (req.getParams().get(UpdateRequest.MIN_REPFACT) != null) {
-// TODO: Kept for rolling upgrades only. Remove in Solr 9
-params.add(UpdateRequest.MIN_REPFACT,
req.getParams().get(UpdateRequest.MIN_REPFACT));
- }
- cmdDistrib.distribDelete(cmd, nodes, params, false,
rollupReplicationTracker, leaderReplicationTracker);
-}
+postProcessDeleteById(cmd);
--- End diff --
Well I could try,
I didn't want to introduce too many changes to the logic. Would changing
this and letting the full sold test suite run be of assurance that the change
is safe?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241945926
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
---
@@ -1417,45 +1407,7 @@ private void doDeleteById(DeleteUpdateCommand cmd)
throws IOException {
return;
}
-if (zkEnabled && isLeader && !isSubShardLeader) {
- DocCollection coll =
zkController.getClusterState().getCollection(collection);
- List subShardLeaders = getSubShardLeaders(coll,
cloudDesc.getShardId(), cmd.getId(), null);
- // the list will actually have only one element for an add
request
- if (subShardLeaders != null && !subShardLeaders.isEmpty()) {
-ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
-params.set(DISTRIB_UPDATE_PARAM,
DistribPhase.FROMLEADER.toString());
-params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
-zkController.getBaseUrl(), req.getCore().getName()));
-params.set(DISTRIB_FROM_PARENT, cloudDesc.getShardId());
-cmdDistrib.distribDelete(cmd, subShardLeaders, params, true, null,
null);
- }
-
- final List nodesByRoutingRules =
getNodesByRoutingRules(zkController.getClusterState(), coll, cmd.getId(), null);
- if (nodesByRoutingRules != null && !nodesByRoutingRules.isEmpty()) {
-ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
-params.set(DISTRIB_UPDATE_PARAM,
DistribPhase.FROMLEADER.toString());
-params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
-zkController.getBaseUrl(), req.getCore().getName()));
-params.set(DISTRIB_FROM_COLLECTION, collection);
-params.set(DISTRIB_FROM_SHARD, cloudDesc.getShardId());
-cmdDistrib.distribDelete(cmd, nodesByRoutingRules, params, true,
null, null);
- }
-}
-
-if (nodes != null) {
- ModifiableSolrParams params = new
ModifiableSolrParams(filterParams(req.getParams()));
- params.set(DISTRIB_UPDATE_PARAM,
- (isLeader || isSubShardLeader ?
DistribPhase.FROMLEADER.toString()
- : DistribPhase.TOLEADER.toString()));
- params.set(DISTRIB_FROM, ZkCoreNodeProps.getCoreUrl(
- zkController.getBaseUrl(), req.getCore().getName()));
-
- if (req.getParams().get(UpdateRequest.MIN_REPFACT) != null) {
-// TODO: Kept for rolling upgrades only. Remove in Solr 9
-params.add(UpdateRequest.MIN_REPFACT,
req.getParams().get(UpdateRequest.MIN_REPFACT));
- }
- cmdDistrib.distribDelete(cmd, nodes, params, false,
rollupReplicationTracker, leaderReplicationTracker);
-}
+postProcessDeleteById(cmd);
--- End diff --
I wonder if we actually need this postPorcessDeleteById(cmd) method...
could the subclass put its logic after calling super.doDeleteById()?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241945549
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java
---
@@ -223,6 +289,7 @@ private String getLeaderUrlZk(String id) {
@Override
boolean isLeader(UpdateCommand cmd) {
+updateCommand = cmd;
--- End diff --
again; this line makes me frown
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241945502
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedStandaloneUpdateProcessor.java
---
@@ -56,8 +59,15 @@ public void doDeleteByQuery(DeleteUpdateCommand cmd)
throws IOException {
super.doDeleteByQuery(cmd, Collections.emptyList(), null);
}
+ @Override
+ protected void postProcessDeleteByQuery(DeleteUpdateCommand cmd,
+ List
replicas, DocCollection coll) throws IOException {
+// no op
+ }
+
@Override
boolean isLeader(UpdateCommand cmd) {
+updateCommand = cmd;
--- End diff --
ouch this piece is ugly. a method like "is..." (or get...) should not
have side effects.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
Github user dsmiley commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/528#discussion_r241945460
--- Diff:
solr/core/src/java/org/apache/solr/update/processor/DistributedZkUpdateProcessor.java
---
@@ -58,6 +75,127 @@ String getCollectionName(CloudDescriptor cloudDesc) {
return cloudDesc.getReplicaType();
}
+ @Override
+ public void doDeleteByQuery(DeleteUpdateCommand cmd) throws IOException {
+// even in non zk mode, tests simulate updates from a leader
--- End diff --
this particular comment is I think intended for the standalone mode
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #528: SOLR-12955 2
GitHub user barrotsteindev opened a pull request: https://github.com/apache/lucene-solr/pull/528 SOLR-12955 2 You can merge this pull request into a Git repository by running: $ git pull https://github.com/barrotsteindev/lucene-solr SOLR-12955-2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/528.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #528 commit d02e74b82f96453c69f2ee72bbd4f55e725ca96d Author: Bar Rotstein Date: 2018-12-13T18:53:36Z SOLR-12955: create two separate procs for zk and non zk distrib update procs commit efbe76ad1de8481ec7dfc6ec8fe684c5aef7f006 Author: Bar Rotstein Date: 2018-12-13T20:20:35Z SOLR-12955: replicaType and collectionName parsing is done separately --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
