You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2020/02/26 09:09:15 UTC
[lucene-solr] 01/03: SOLR-14275: attempt to reduce the complexity.
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch jira/solr-14275
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit f65ea6e13326de3f6282e07ae401f66dbb95d764
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Tue Feb 25 20:47:53 2020 +0100
SOLR-14275: attempt to reduce the complexity.
---
.../cloud/autoscaling/InactiveShardPlanAction.java | 13 +++++--
.../cloud/autoscaling/sim/SimCloudManager.java | 2 +-
.../cloud/autoscaling/AddReplicaSuggester.java | 2 +-
.../client/solrj/cloud/autoscaling/Clause.java | 7 +++-
.../solr/client/solrj/cloud/autoscaling/Row.java | 41 ++++++++++++----------
.../client/solrj/cloud/autoscaling/Suggester.java | 11 ++++--
6 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/InactiveShardPlanAction.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/InactiveShardPlanAction.java
index 6fca29a..e6f228a 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/InactiveShardPlanAction.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/InactiveShardPlanAction.java
@@ -18,6 +18,7 @@ package org.apache.solr.cloud.autoscaling;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -26,6 +27,7 @@ import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.cloud.DistribStateManager;
import org.apache.solr.client.solrj.cloud.SolrCloudManager;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.common.cloud.ClusterState;
@@ -102,9 +104,14 @@ public class InactiveShardPlanAction extends TriggerActionBase {
String parentPath = ZkStateReader.COLLECTIONS_ZKNODE + "/" + coll.getName();
List<String> locks;
try {
- locks = cloudManager.getDistribStateManager().listData(parentPath).stream()
- .filter(name -> name.endsWith("-splitting"))
- .collect(Collectors.toList());
+ DistribStateManager stateManager = cloudManager.getDistribStateManager();
+ if (stateManager.hasData(parentPath)) {
+ locks = cloudManager.getDistribStateManager().listData(parentPath).stream()
+ .filter(name -> name.endsWith("-splitting"))
+ .collect(Collectors.toList());
+ } else {
+ locks = Collections.emptyList();
+ }
for (String lock : locks) {
try {
String lockPath = parentPath + "/" + lock;
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java
index 3bcc273..87dee15 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimCloudManager.java
@@ -729,7 +729,7 @@ public class SimCloudManager implements SolrCloudManager {
// by the testcase. we're going to immediately catch & re-throw any exceptions, so we don't
// need/want to be wrapped in a LoggingCallable w/getBackgroundTaskFailureCount() tracking
Future<SolrResponse> rsp = simCloudManagerPool.submit(() -> simHandleSolrRequest(req));
- return rsp.get(120, TimeUnit.SECONDS); // longer then this and something is seriously wrong
+ return rsp.get(6000, TimeUnit.SECONDS); // longer then this and something is seriously wrong
} catch (Exception e) {
throw new IOException(e);
}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java
index 87b831a..2e910db 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java
@@ -51,7 +51,7 @@ class AddReplicaSuggester extends Suggester {
Row row = getMatrix().get(i);
if (!isNodeSuitableForReplicaAddition(row, null)) continue;
Row tmpRow = row.addReplica(shard.first(), shard.second(), type, strict);
- List<Violation> errs = testChangedMatrix(strict, tmpRow.session);
+ List<Violation> errs = testChangedMatrix(strict, tmpRow, tmpRow.session);
if (!containsNewErrors(errs)) {
if ((errs.isEmpty() && isLessDeviant()) ||//there are no violations but this is deviating less
isLessSerious(errs, leastSeriousViolation)) {//there are errors , but this has less serious violation
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
index f12ecce..5bdb46f 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
@@ -658,6 +658,10 @@ public class Clause implements MapWriter, Comparable<Clause> {
}
public List<Violation> test(Policy.Session session, double[] deviations) {
+ return test(session, null, deviations);
+ }
+
+ public List<Violation> test(Policy.Session session, Row changedRow, double[] deviations) {
if (isPerCollectiontag()) {
if(nodeSetPresent) {
if(put == Put.ON_EACH){
@@ -674,7 +678,8 @@ public class Clause implements MapWriter, Comparable<Clause> {
} else {
ComputedValueEvaluator computedValueEvaluator = new ComputedValueEvaluator(session);
Violation.Ctx ctx = new Violation.Ctx(this, session.matrix, computedValueEvaluator);
- for (Row r : session.matrix) {
+ Collection<Row> rows = changedRow != null ? Collections.singleton(changedRow) : session.matrix;
+ for (Row r : rows) {
computedValueEvaluator.node = r.node;
SealedClause sealedClause = getSealedClause(computedValueEvaluator);
if (!sealedClause.getGlobalTag().isPass(r)) {
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
index 2cc48ea..a516de4 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
@@ -27,6 +27,7 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Random;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
@@ -120,33 +121,37 @@ public class Row implements MapWriter {
public <R> R computeCacheIfAbsent(String cacheName, Function<Object, R> supplier) {
- R result = (R) globalCache.get(cacheName);
- if (result != null) {
- assert CacheEntry.hit(cacheName);
- return result;
+ AtomicBoolean created = new AtomicBoolean();
+ R result = (R) globalCache.computeIfAbsent(cacheName, c -> {
+ created.set(true);
+ return supplier.apply(cacheName);
+ });
+ if (created.get()) {
+ CacheEntry.miss(cacheName);
} else {
- assert CacheEntry.miss(cacheName);
- globalCache.put(cacheName, result = supplier.apply(cacheName));
- return result;
+ CacheEntry.hit(cacheName);
}
+ return result;
}
public <R> R computeCacheIfAbsent(String coll, String shard, String cacheName, Object key, Function<Object, R> supplier) {
- Map collMap = (Map) this.perCollCache.get(coll);
- if (collMap == null) this.perCollCache.put(coll, collMap = new HashMap());
- Map shardMap = (Map) collMap.get(shard);
- if (shardMap == null) collMap.put(shard, shardMap = new HashMap());
- Map cacheNameMap = (Map) shardMap.get(cacheName);
- if (cacheNameMap == null) shardMap.put(cacheName, cacheNameMap = new HashMap());
- R result = (R) cacheNameMap.get(key);
- if (result == null) {
+ Map collMap = (Map) this.perCollCache.computeIfAbsent(coll, c -> new HashMap());
+ //if (collMap == null) this.perCollCache.put(coll, collMap = new HashMap());
+ Map shardMap = (Map) collMap.computeIfAbsent(shard, s -> new HashMap<>());
+// if (shardMap == null) collMap.put(shard, shardMap = new HashMap());
+ Map cacheNameMap = (Map) shardMap.computeIfAbsent(cacheName, c -> new HashMap<>());
+// if (cacheNameMap == null) shardMap.put(cacheName, cacheNameMap = new HashMap());
+ AtomicBoolean created = new AtomicBoolean();
+ R result = (R) cacheNameMap.computeIfAbsent(key, k -> {
+ created.set(true);
+ return supplier.apply(k);
+ });
+ if (created.get()) {
CacheEntry.miss(cacheName);
- cacheNameMap.put(key, result = supplier.apply(key));
- return result;
} else {
CacheEntry.hit(cacheName);
- return result;
}
+ return result;
}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java
index 22da717..bbaabf2 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java
@@ -120,6 +120,7 @@ public abstract class Suggester implements MapWriter {
return this;
}
+ // is this addition breaking any policy rules?
protected boolean isNodeSuitableForReplicaAddition(Row targetRow, Row srcRow) {
if (!targetRow.isLive) return false;
if (!isAllowed(targetRow.node, Hint.TARGET_NODE)) return false;
@@ -322,15 +323,21 @@ public abstract class Suggester implements MapWriter {
}
List<Violation> testChangedMatrix(boolean executeInStrictMode, Policy.Session session) {
+ return testChangedMatrix(executeInStrictMode, null, session);
+ }
+
+ List<Violation> testChangedMatrix(boolean executeInStrictMode, Row changedRow, Policy.Session session) {
if (this.deviations != null) this.lastBestDeviation = this.deviations;
this.deviations = null;
- Policy.setApproxValuesAndSortNodes(session.getPolicy().clusterPreferences, session.matrix);
+ if (changedRow != null) {
+ Policy.setApproxValuesAndSortNodes(session.getPolicy().clusterPreferences, session.matrix);
+ }
List<Violation> errors = new ArrayList<>();
for (Clause clause : session.expandedClauses) {
Clause originalClause = clause.derivedFrom == null ? clause : clause.derivedFrom;
if (this.deviations == null) this.deviations = new LinkedHashMap<>();
this.deviations.put(originalClause, new double[1]);
- List<Violation> errs = clause.test(session, this.deviations == null ? null : this.deviations.get(originalClause));
+ List<Violation> errs = clause.test(session, changedRow, this.deviations == null ? null : this.deviations.get(originalClause));
if (!errs.isEmpty() &&
(executeInStrictMode || clause.strict)) errors.addAll(errs);
}