SOLR-14347: Autoscaling placement wrong when concurrent replica placements are calculated.

This commit is contained in:
Andrzej Bialecki 2020-03-23 18:21:40 +01:00
parent 7f460faffb
commit 68e4304453
9 changed files with 73 additions and 55 deletions

View File

@ -72,6 +72,8 @@ Bug Fixes
* SOLR-13199: Return proper error when invalid parentFilter is passed in ChildDocTransformer
(Johannes Kloos, Munendra S N, David Smiley, Mikhail Khludnev)
* SOLR-14347: Autoscaling placement wrong when concurrent replica placements are calculated. (ab)
Other Changes
---------------------
* SOLR-14197: SolrResourceLoader: marked many methods as deprecated, and in some cases rerouted exiting logic to avoid

View File

@ -315,7 +315,7 @@ public class SimCloudManager implements SolrCloudManager {
config = cloudManager.getDistribStateManager().getAutoScalingConfig();
}
Set<String> nodeTags = new HashSet<>(SimUtils.COMMON_NODE_TAGS);
nodeTags.addAll(config.getPolicy().getParams());
nodeTags.addAll(config.getPolicy().getParamNames());
Set<String> replicaTags = new HashSet<>(SimUtils.COMMON_REPLICA_TAGS);
replicaTags.addAll(config.getPolicy().getPerReplicaAttributes());
cloudManager.getSimClusterStateProvider().copyFrom(other.getClusterStateProvider());

View File

@ -53,7 +53,7 @@ public class SnapshotNodeStateProvider implements NodeStateProvider {
config = other.getDistribStateManager().getAutoScalingConfig();
}
Set<String> nodeTags = new HashSet<>(SimUtils.COMMON_NODE_TAGS);
nodeTags.addAll(config.getPolicy().getParams());
nodeTags.addAll(config.getPolicy().getParamNames());
Set<String> replicaTags = new HashSet<>(SimUtils.COMMON_REPLICA_TAGS);
replicaTags.addAll(config.getPolicy().getPerReplicaAttributes());
for (String node : other.getClusterStateProvider().getLiveNodes()) {

View File

@ -36,12 +36,10 @@ import org.apache.solr.common.cloud.Slice;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Ignore("SOLR-13884")
public class ConcurrentCreateCollectionTest extends SolrCloudTestCase {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@ -51,8 +49,8 @@ public class ConcurrentCreateCollectionTest extends SolrCloudTestCase {
@BeforeClass
public static void setupCluster() throws Exception {
configureCluster(NODES)
// .addConfig("conf", configset("cloud-minimal"))
.addConfig("conf", configset("_default"))
.addConfig("conf", configset("cloud-minimal"))
//.addConfig("conf", configset("_default"))
.configure();
}

View File

@ -72,8 +72,8 @@ public class MoveReplicaSuggester extends Suggester {
int result = -1;
if (!force && srcRowModified.isLive && targetRow.isLive) {
result = tmpSession.getPolicy().clusterPreferences.get(0).compare(srcRowModified, tmpSession.getNode(targetRow.node), true);
if (result == 0) result = tmpSession.getPolicy().clusterPreferences.get(0).compare(srcRowModified, tmpSession.getNode(targetRow.node), false);
result = tmpSession.getPolicy().getClusterPreferences().get(0).compare(srcRowModified, tmpSession.getNode(targetRow.node), true);
if (result == 0) result = tmpSession.getPolicy().getClusterPreferences().get(0).compare(srcRowModified, tmpSession.getNode(targetRow.node), false);
}
if (result <= 0) {

View File

@ -91,21 +91,21 @@ public class Policy implements MapWriter {
*/
private static final List<String> DEFAULT_PARAMS_OF_INTEREST = Arrays.asList(ImplicitSnitch.DISK, ImplicitSnitch.CORES);
final Map<String, List<Clause>> policies;
final List<Clause> clusterPolicy;
final List<Preference> clusterPreferences;
final List<Pair<String, Type>> params;
final List<String> perReplicaAttributes;
final int zkVersion;
private final Map<String, List<Clause>> policies;
private final List<Clause> clusterPolicy;
private final List<Preference> clusterPreferences;
private final List<Pair<String, Type>> params;
private final List<String> perReplicaAttributes;
private final int zkVersion;
/**
* True if cluster policy, preferences and custom policies are all non-existent
*/
final boolean empty;
private final boolean empty;
/**
* True if cluster preferences was originally empty, false otherwise. It is used to figure out if
* the current preferences were implicitly added or not.
*/
final boolean emptyPreferences;
private final boolean emptyPreferences;
public Policy() {
this(Collections.emptyMap());
@ -215,11 +215,11 @@ public class Policy implements MapWriter {
}
public List<Clause> getClusterPolicy() {
return clusterPolicy;
return Collections.unmodifiableList(clusterPolicy);
}
public List<Preference> getClusterPreferences() {
return clusterPreferences;
return Collections.unmodifiableList(clusterPreferences);
}
@Override
@ -355,7 +355,11 @@ public class Policy implements MapWriter {
}
public Session createSession(SolrCloudManager cloudManager) {
return createSession(cloudManager, null);
return new Session(cloudManager, this, null);
}
public Session createSession(SolrCloudManager cloudManager, Transaction tx) {
return new Session(cloudManager, this, tx);
}
public enum SortParam {
@ -392,10 +396,6 @@ public class Policy implements MapWriter {
}
}
private Session createSession(SolrCloudManager cloudManager, Transaction tx) {
return new Session(cloudManager, tx);
}
public static List<Clause> mergePolicies(String coll,
List<Clause> collPolicy,
List<Clause> globalPolicy) {
@ -462,7 +462,11 @@ public class Policy implements MapWriter {
return policies;
}
public List<String> getParams() {
public List<Pair<String, Type>> getParams() {
return Collections.unmodifiableList(params);
}
public List<String> getParamNames() {
return params.stream().map(Pair::first).collect(toList());
}
@ -470,6 +474,10 @@ public class Policy implements MapWriter {
return Collections.unmodifiableList(perReplicaAttributes);
}
public int getZkVersion() {
return zkVersion;
}
/**
* Compares two {@link Row} loads according to a policy.
*
@ -503,33 +511,22 @@ public class Policy implements MapWriter {
* a cluster state.
*
*/
public class Session implements MapWriter {
public static class Session implements MapWriter {
final List<String> nodes;
final SolrCloudManager cloudManager;
final List<Row> matrix;
final NodeStateProvider nodeStateProvider;
final int znodeVersion;
Set<String> collections = new HashSet<>();
final Policy policy;
List<Clause> expandedClauses;
List<Violation> violations = new ArrayList<>();
Transaction transaction;
private Session(List<String> nodes, SolrCloudManager cloudManager,
List<Row> matrix, List<Clause> expandedClauses, int znodeVersion,
NodeStateProvider nodeStateProvider, Transaction transaction) {
this.transaction = transaction;
this.nodes = nodes;
this.cloudManager = cloudManager;
this.matrix = matrix;
this.expandedClauses = expandedClauses;
this.znodeVersion = znodeVersion;
this.nodeStateProvider = nodeStateProvider;
for (Row row : matrix) row.session = this;
}
Session(SolrCloudManager cloudManager, Transaction transaction) {
Session(SolrCloudManager cloudManager, Policy policy, Transaction transaction) {
this.transaction = transaction;
this.policy = policy;
ClusterState state = null;
this.nodeStateProvider = cloudManager.getNodeStateProvider();
try {
@ -545,7 +542,7 @@ public class Policy implements MapWriter {
collections.addAll(nodeStateProvider.getReplicaInfo(node, Collections.emptyList()).keySet());
}
expandedClauses = clusterPolicy.stream()
expandedClauses = policy.getClusterPolicy().stream()
.filter(clause -> !clause.isPerCollectiontag())
.collect(Collectors.toList());
@ -566,29 +563,48 @@ public class Policy implements MapWriter {
ClusterStateProvider stateProvider = cloudManager.getClusterStateProvider();
for (String c : collections) {
addClausesForCollection(stateProvider, c);
addClausesForCollection(policy, expandedClauses, stateProvider, c);
}
Collections.sort(expandedClauses);
matrix = new ArrayList<>(nodes.size());
for (String node : nodes) matrix.add(new Row(node, params, perReplicaAttributes, this));
for (String node : nodes) matrix.add(new Row(node, policy.getParams(), policy.getPerReplicaAttributes(), this));
applyRules();
}
void addClausesForCollection(ClusterStateProvider stateProvider, String c) {
private Session(List<String> nodes, SolrCloudManager cloudManager,
List<Row> matrix, List<Clause> expandedClauses, int znodeVersion,
NodeStateProvider nodeStateProvider, Policy policy, Transaction transaction) {
this.transaction = transaction;
this.policy = policy;
this.nodes = nodes;
this.cloudManager = cloudManager;
this.matrix = matrix;
this.expandedClauses = expandedClauses;
this.znodeVersion = znodeVersion;
this.nodeStateProvider = nodeStateProvider;
for (Row row : matrix) row.session = this;
}
void addClausesForCollection(ClusterStateProvider stateProvider, String collection) {
addClausesForCollection(policy, expandedClauses, stateProvider, collection);
}
public static void addClausesForCollection(Policy policy, List<Clause> clauses, ClusterStateProvider stateProvider, String c) {
String p = stateProvider.getPolicyNameByCollection(c);
if (p != null) {
List<Clause> perCollPolicy = policies.get(p);
List<Clause> perCollPolicy = policy.getPolicies().get(p);
if (perCollPolicy == null) {
return;
}
}
expandedClauses.addAll(mergePolicies(c, policies.getOrDefault(p, emptyList()), clusterPolicy));
clauses.addAll(mergePolicies(c, policy.getPolicies().getOrDefault(p, emptyList()), policy.getClusterPolicy()));
}
Session copy() {
return new Session(nodes, cloudManager, getMatrixCopy(), expandedClauses, znodeVersion, nodeStateProvider, transaction);
return new Session(nodes, cloudManager, getMatrixCopy(), expandedClauses, znodeVersion, nodeStateProvider, policy, transaction);
}
public Row getNode(String node) {
@ -603,7 +619,7 @@ public class Policy implements MapWriter {
}
public Policy getPolicy() {
return Policy.this;
return policy;
}
@ -620,7 +636,7 @@ public class Policy implements MapWriter {
}
void sortNodes() {
setApproxValuesAndSortNodes(clusterPreferences, matrix);
setApproxValuesAndSortNodes(policy.getClusterPreferences(), matrix);
}

View File

@ -121,7 +121,7 @@ public class PolicyHelper {
policyMapping.set(optionalPolicyMapping);
SessionWrapper sessionWrapper = null;
Policy.Session session = null;
Policy.Session origSession = null;
try {
try {
SESSION_WRAPPPER_REF.set(sessionWrapper = getSession(delegatingManager));
@ -129,7 +129,9 @@ public class PolicyHelper {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "unable to get autoscaling policy session", e);
}
session = sessionWrapper.session;
origSession = sessionWrapper.session;
// new session needs to be created to avoid side-effects from per-collection policies
Policy.Session session = new Policy.Session(delegatingManager, origSession.policy, origSession.transaction);
Map<String, Double> diskSpaceReqd = new HashMap<>();
try {
DocCollection coll = cloudManager.getClusterStateProvider().getCollection(collName);
@ -193,7 +195,7 @@ public class PolicyHelper {
} finally {
policyMapping.remove();
if (sessionWrapper != null) {
sessionWrapper.returnSession(session);
sessionWrapper.returnSession(origSession);
}
}
return positions;
@ -563,7 +565,7 @@ public class PolicyHelper {
this.ref = ref;
this.zkVersion = session == null ?
0 :
session.getPolicy().zkVersion;
session.getPolicy().getZkVersion();
}
public Policy.Session get() {

View File

@ -177,7 +177,7 @@ public abstract class Suggester implements MapWriter {
// the source node is dead so live nodes may not have it
for (String srcNode : srcNodes) {
if (session.matrix.stream().noneMatch(row -> row.node.equals(srcNode))) {
session.matrix.add(new Row(srcNode, session.getPolicy().params, session.getPolicy().perReplicaAttributes, session));
session.matrix.add(new Row(srcNode, session.getPolicy().getParams(), session.getPolicy().getPerReplicaAttributes(), session));
}
}
}
@ -324,7 +324,7 @@ public abstract class Suggester implements MapWriter {
List<Violation> testChangedMatrix(boolean executeInStrictMode, Policy.Session session) {
if (this.deviations != null) this.lastBestDeviation = this.deviations;
this.deviations = null;
Policy.setApproxValuesAndSortNodes(session.getPolicy().clusterPreferences, session.matrix);
Policy.setApproxValuesAndSortNodes(session.getPolicy().getClusterPreferences(), session.matrix);
List<Violation> errors = new ArrayList<>();
for (Clause clause : session.expandedClauses) {
Clause originalClause = clause.derivedFrom == null ? clause : clause.derivedFrom;

View File

@ -2757,7 +2757,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
if (!row.isLive) deadNodes++;
}
Policy.setApproxValuesAndSortNodes(policy.clusterPreferences, rows);
Policy.setApproxValuesAndSortNodes(policy.getClusterPreferences(), rows);
for (int i = 0; i < deadNodes; i++) {
assertFalse(rows.get(i).isLive);