You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/10/31 00:20:25 UTC
[lucene-solr] branch reference_impl_dev updated: @1091 Lovin'
tweaks.
This is an automated email from the ASF dual-hosted git repository.
markrmiller pushed a commit to branch reference_impl_dev
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/reference_impl_dev by this push:
new dc11954 @1091 Lovin' tweaks.
dc11954 is described below
commit dc119545ab34f27ec54fe9fec01e663adce92267
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Fri Oct 30 19:20:02 2020 -0500
@1091 Lovin' tweaks.
---
.../java/org/apache/solr/cloud/LeaderElector.java | 7 ++++++-
.../src/java/org/apache/solr/cloud/Overseer.java | 2 +-
.../java/org/apache/solr/cloud/ZkController.java | 6 +++---
.../org/apache/solr/cloud/ZkDistributedQueue.java | 22 +++++++++++-----------
.../cloud/api/collections/CreateCollectionCmd.java | 2 +-
.../solr/cloud/api/collections/SplitShardCmd.java | 15 ---------------
.../apache/solr/core/CorePropertiesLocator.java | 4 ----
.../solr/handler/admin/CoreAdminOperation.java | 5 +++--
.../CollectionsAPIDistClusterPerZkTest.java | 9 +++++++--
.../solr/handler/admin/CoreAdminHandlerTest.java | 4 ++--
.../org/apache/solr/search/TestXmlQParser.java | 11 +++++++++++
.../solrj/request/CollectionAdminRequest.java | 14 +++++++-------
.../apache/solr/cloud/MiniSolrCloudCluster.java | 3 ++-
13 files changed, 54 insertions(+), 50 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cloud/LeaderElector.java b/solr/core/src/java/org/apache/solr/cloud/LeaderElector.java
index 59a4d6e..7b25968 100644
--- a/solr/core/src/java/org/apache/solr/cloud/LeaderElector.java
+++ b/solr/core/src/java/org/apache/solr/cloud/LeaderElector.java
@@ -110,7 +110,12 @@ public class LeaderElector {
List<String> seqs = zkClient.getChildren(holdElectionPath, null, true);
sortSeqs(seqs);
- String leaderSeqNodeName = context.leaderSeqPath.substring(context.leaderSeqPath.lastIndexOf('/') + 1);
+ String leaderSeqNodeName;
+ try {
+ leaderSeqNodeName = context.leaderSeqPath.substring(context.leaderSeqPath.lastIndexOf('/') + 1);
+ } catch (NullPointerException e) {
+ return false;
+ }
if (!seqs.contains(leaderSeqNodeName)) {
log.warn("Our node is no longer in line to be leader");
return false;
diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
index 148a2ef..a277a42 100644
--- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java
+++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
@@ -361,7 +361,7 @@ public class Overseer implements SolrCloseable {
// if an event comes in the next *ms batch it together
int wait = 10;
if (log.isDebugEnabled()) log.debug("going to peekElements processedNodes={}", processedNodes);
- queue = new LinkedList<>(stateUpdateQueue.peekElements(100, wait, node -> processedNodes.contains(node)));
+ queue = new LinkedList<>(stateUpdateQueue.peekElements(10, wait, node -> processedNodes.contains(node)));
}
fallbackQueueSize = processedNodes.size();
// we should force write all pending updates because the next iteration might sleep until there
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
index 4f4b732..ab3339c 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -1034,7 +1034,7 @@ public class ZkController implements Closeable, Runnable {
DistributedLock lock = new DistributedLock(zkClient, "/cluster/cluster_lock", zkClient.getZkACLProvider().getACLsToAdd("/cluster/cluster_lock"));
if (log.isDebugEnabled()) log.debug("get cluster lock");
while (!lock.lock()) {
- Thread.sleep(50);
+ Thread.sleep(150);
}
try {
@@ -1113,7 +1113,7 @@ public class ZkController implements Closeable, Runnable {
} finally {
if (log.isDebugEnabled()) log.debug("release cluster lock");
- lock.unlock();
+ if (lock.isOwner()) lock.unlock();
}
if (!createdClusterNodes) {
// wait?
@@ -1138,7 +1138,7 @@ public class ZkController implements Closeable, Runnable {
this.sysPropsCacher = new NodesSysPropsCacher(getSolrCloudManager().getNodeStateProvider(),
getNodeName(), zkStateReader);
- try (ParWork worker = new ParWork((this))) {
+ try (ParWork worker = new ParWork(this, false, true)) {
// start the overseer first as following code may need it's processing
worker.collect("startOverseer", () -> {
LeaderElector overseerElector = new LeaderElector(zkClient, new ContextKey("overseer", "overseer"), overseerContexts);
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java
index 670a23d..b29768c 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java
@@ -543,19 +543,20 @@ public class ZkDistributedQueue implements DistributedQueue {
if (log.isDebugEnabled()) log.debug("0 wait time and no children found, return");
return;
}
-
- while (foundChildren.size() == 0) {
+ TreeSet<String> fc = null;
+ while (fc == null || fc.isEmpty()) {
if (watcher.fired) {
watcher.fired = false;
- foundChildren = fetchZkChildren(watcher, acceptFilter);
- if (!foundChildren.isEmpty()) {
- break;
+ fc = fetchZkChildren(watcher, acceptFilter);
+ if (!fc.isEmpty()) {
+ foundChildren.addAll(fc);
+ return;
}
}
updateLock.lockInterruptibly();
try {
try {
- changed.await(250, TimeUnit.MILLISECONDS);
+ changed.await(10, TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
ParWork.propagateInterrupt(e);
}
@@ -566,19 +567,18 @@ public class ZkDistributedQueue implements DistributedQueue {
return;
}
for (String child : knownChildren) {
- if (acceptFilter == null || !acceptFilter.test(dir + "/" + child)) {
+ if (acceptFilter == null || !acceptFilter.test(child)) {
foundChildren.add(child);
}
}
+ if (!foundChildren.isEmpty()) {
+ return;
+ }
} finally {
if (updateLock.isHeldByCurrentThread()) {
updateLock.unlock();
}
}
- if (!foundChildren.isEmpty()) {
- break;
- }
-
}
}
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
index bb8ee8a..4ae8fb6 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
@@ -793,7 +793,7 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
replicas++;
}
}
- if (replicas == expectedReplicas) {
+ if (replicas >= expectedReplicas) {
log.info("Found expected replicas={} {}", expectedReplicas, replicaMap);
return true;
}
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
index e83bd06..cdc2555 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
@@ -854,21 +854,6 @@ public class SplitShardCmd implements OverseerCollectionMessageHandler.Cmd {
}
} else {
parentSlice = collection.getSlice(slice.get());
- int tryCnt = 0;
- // TODO use zkStateReader#waitFor
- while (parentSlice.getState() != Slice.State.ACTIVE && tryCnt < 500) {
- tryCnt++;
- try {
- Thread.sleep(100);
- } catch (InterruptedException e) {
- ParWork.propagateInterrupt(e);
- }
- clusterState = zkStateReader.getClusterState();
- collection = clusterState.getCollection(collectionName);
-
- parentSlice = collection.getSlice(slice.get());
- }
-
}
if (parentSlice == null) {
diff --git a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
index 1c5ce26..4e4e2f8 100644
--- a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
+++ b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
@@ -63,10 +63,6 @@ public class CorePropertiesLocator implements CoresLocator {
public void create(CoreContainer cc, CoreDescriptor... coreDescriptors) {
for (CoreDescriptor cd : coreDescriptors) {
Path propertiesFile = cd.getInstanceDir().resolve(PROPERTIES_FILENAME);
- if (Files.exists(propertiesFile))
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "Could not create a new core in " + cd.getInstanceDir()
- + " as another core is already defined there");
writePropertiesFile(cd, propertiesFile);
}
}
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
index aacbe2f..cf9e6b6 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
@@ -369,12 +369,13 @@ enum CoreAdminOperation implements CoreAdminOp {
public void execute(CallInfo it) throws Exception {
try {
fun.execute(it);
- } catch (SolrException | InterruptedException e) {
+ } catch (InterruptedException e) {
ParWork.propagateInterrupt(e);
// No need to re-wrap; throw as-is.
throw e;
} catch (Exception e) {
- throw new SolrException(ErrorCode.SERVER_ERROR, "Error handling '" + action.name() + "' action", e);
+ log.error("", e);
+ throw new SolrException(ErrorCode.SERVER_ERROR, "Error handling '" + action.name() + "' action. " + e.getMessage(), e);
}
}
}
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistClusterPerZkTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistClusterPerZkTest.java
index f670ad0..2325baf 100644
--- a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistClusterPerZkTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionsAPIDistClusterPerZkTest.java
@@ -289,8 +289,13 @@ public class CollectionsAPIDistClusterPerZkTest extends SolrCloudTestCase {
colls.add(coll);
}
- for (Coll coll : colls) {
- cluster.waitForActiveCollection(coll.name, coll.numShards, coll.numShards * coll.replicationFactor);
+ for (CollectionAdminRequest.Create create : createRequests) {
+ if (create == null) continue;
+ try {
+ cluster.waitForActiveCollection(create.getCollectionName(), create.getNumShards(), create.getNumShards() * (create.getNumNrtReplicas() + create.getNumTlogReplicas() + create.getNumPullReplicas()));
+ } catch(Exception e) {
+ throw new RuntimeException(create.getParams().toString(), e);
+ }
}
for (int i = 0; i < cluster.getJettySolrRunners().size(); i++) {
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
index 75c5716..1a10911 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
@@ -411,7 +411,7 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
CoreAdminParams.CORE, "non-existent-core")
, resp);
});
- assertEquals("Expected error message for non-existent core.", "No such core: non-existent-core", e.getMessage());
+ assertTrue("Expected error message for non-existent core: " + e.getMessage(), e.getMessage().contains("No such core: non-existent-core"));
// test null core
e = expectThrows(SolrException.class, () -> {
@@ -420,7 +420,7 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
CoreAdminParams.CoreAdminAction.RELOAD.toString())
, resp);
});
- assertEquals("Expected error message for non-existent core.", "Missing required parameter: core", e.getMessage());
+ assertTrue("Expected error message for non-existent core.", e.getMessage().contains("Missing required parameter: core"));
admin.close();
}
}
diff --git a/solr/core/src/test/org/apache/solr/search/TestXmlQParser.java b/solr/core/src/test/org/apache/solr/search/TestXmlQParser.java
index f13ec57..f353ecf 100644
--- a/solr/core/src/test/org/apache/solr/search/TestXmlQParser.java
+++ b/solr/core/src/test/org/apache/solr/search/TestXmlQParser.java
@@ -16,6 +16,7 @@
*/
package org.apache.solr.search;
+import java.io.InputStream;
import java.lang.invoke.MethodHandles;
import org.apache.lucene.queryparser.xml.CoreParser;
@@ -23,6 +24,7 @@ import org.apache.lucene.queryparser.xml.CoreParser;
import org.apache.lucene.queryparser.xml.TestCoreParser;
import org.apache.solr.util.StartupLoggingUtils;
import org.junit.AfterClass;
+import org.junit.BeforeClass;
import org.junit.Ignore;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -32,6 +34,15 @@ public class TestXmlQParser extends TestCoreParser {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private CoreParser solrCoreParser;
+ @BeforeClass
+ public static void beforeTestXmlQParser() throws Exception {
+ String resource = "PointRangeQueryWithoutLowerTerm.xml";
+ try(InputStream xmlStream = TestCoreParser.class.getResourceAsStream(resource)) {
+ assumeTrue("Test XML file " + resource + " cannot be found - "
+ + "these resources are in a lucene module and some IDE's or test runners may not find them", xmlStream != null);
+ }
+ }
+
@AfterClass
public static void shutdownLogger() throws Exception {
StartupLoggingUtils.shutdown();
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
index 6d738de..eac3ec0 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
@@ -370,7 +370,7 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
* @param numReplicas the replication factor of the collection (same as numNrtReplicas)
*/
public static Create createCollection(String collection, String config, int numShards, int numReplicas) {
- return new Create(collection, config, numShards, numReplicas, null, null);
+ return new Create(collection, config, numShards, numReplicas, 0, 0);
}
/**
@@ -429,11 +429,11 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
protected String policy;
protected String shards;
protected String routerField;
- protected Integer numShards;
+ protected Integer numShards = 0;
protected Integer maxShardsPerNode;
- protected Integer nrtReplicas;
- protected Integer pullReplicas;
- protected Integer tlogReplicas;
+ protected Integer nrtReplicas = 0;
+ protected Integer pullReplicas = 0;
+ protected Integer tlogReplicas = 0;
protected Properties properties;
protected String alias;
@@ -446,13 +446,13 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
/** Constructor that assumes {@link ImplicitDocRouter#NAME} and an explicit list of <code>shards</code> */
protected Create(String collection, String config, String shards, int numNrtReplicas) {
- this(collection, config, ImplicitDocRouter.NAME, null, checkNotNull("shards",shards), numNrtReplicas, null, null);
+ this(collection, config, ImplicitDocRouter.NAME, 0, checkNotNull("shards",shards), numNrtReplicas, 0, 0);
}
private Create(String collection, String config, String routerName, Integer numShards, String shards, Integer numNrtReplicas, Integer numTlogReplicas, Integer numPullReplicas) {
super(CollectionAction.CREATE, SolrIdentifierValidator.validateCollectionName(collection));
// NOTE: there's very little we can assert about the args because nothing but "collection" is required by the server
- if ((null != shards) && (null != numShards)) {
+ if ((null != shards) && (null != numShards && numShards != 0)) {
throw new IllegalArgumentException("Can not specify both a numShards and a list of shards");
}
this.configName = config;
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java
index db32fa2..c4ea258 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java
@@ -616,7 +616,7 @@ public class MiniSolrCloudCluster {
try {
CollectionAdminRequest.deleteCollection(collection).process(solrClient);
} catch (BaseHttpSolrClient.RemoteSolrException e) {
- if (!e.getMessage().contains("Could not find")) {
+ if (!e.getMessage().contains("Could not find") && !e.getMessage().contains("Error handling 'UNLOAD' action")) {
errors.put(collection, e);
}
} catch (Exception e) {
@@ -946,6 +946,7 @@ public class MiniSolrCloudCluster {
}
public void waitForActiveCollection(String collection, int shards, int totalReplicas) {
+ if (collection == null) throw new IllegalArgumentException("null collection");
waitForActiveCollection(collection, 10, TimeUnit.SECONDS, shards, totalReplicas);
}