mirror of https://github.com/apache/lucene.git
SOLR-13045: Sim node versioning should start at 0
Prior to this commit, new ZK nodes being simulated by the sim framework were started with a version of -1. This causes problems, since -1 is also coincidentally the flag value used to ignore optimistic concurrency locking and force overwrite values.
This commit is contained in:
parent
272178eff5
commit
207b3f4453
|
@ -70,7 +70,7 @@ public class OverseerTriggerThread implements Runnable, SolrCloseable {
|
|||
/*
|
||||
Following variables are only accessed or modified when updateLock is held
|
||||
*/
|
||||
private int znodeVersion = -1;
|
||||
private int znodeVersion = 0;
|
||||
|
||||
private Map<String, AutoScaling.Trigger> activeTriggers = new HashMap<>();
|
||||
|
||||
|
|
|
@ -631,10 +631,10 @@ public class SimClusterStateProvider implements ClusterStateProvider {
|
|||
byte[] data = Utils.toJSON(state);
|
||||
try {
|
||||
VersionedData oldData = stateManager.getData(ZkStateReader.CLUSTER_STATE);
|
||||
int version = oldData != null ? oldData.getVersion() : -1;
|
||||
Assert.assertEquals(clusterStateVersion, version + 1);
|
||||
int version = oldData != null ? oldData.getVersion() : 0;
|
||||
Assert.assertEquals(clusterStateVersion, version);
|
||||
stateManager.setData(ZkStateReader.CLUSTER_STATE, data, version);
|
||||
log.debug("** saved cluster state version " + (version + 1));
|
||||
log.debug("** saved cluster state version " + (version));
|
||||
clusterStateVersion++;
|
||||
} catch (Exception e) {
|
||||
throw new IOException(e);
|
||||
|
|
|
@ -72,7 +72,7 @@ public class SimDistribStateManager implements DistribStateManager {
|
|||
|
||||
public static final class Node {
|
||||
ReentrantLock dataLock = new ReentrantLock();
|
||||
private int version = -1;
|
||||
private int version = 0;
|
||||
private int seq = 0;
|
||||
private final CreateMode mode;
|
||||
private final String clientId;
|
||||
|
@ -90,7 +90,11 @@ public class SimDistribStateManager implements DistribStateManager {
|
|||
this.path = path;
|
||||
this.mode = mode;
|
||||
this.clientId = clientId;
|
||||
}
|
||||
|
||||
Node(Node parent, String name, String path, byte[] data, CreateMode mode, String clientId) {
|
||||
this(parent, name, path, mode, clientId);
|
||||
this.data = data;
|
||||
}
|
||||
|
||||
public void clear() {
|
||||
|
@ -311,7 +315,7 @@ public class SimDistribStateManager implements DistribStateManager {
|
|||
n = parentNode.children != null ? parentNode.children.get(currentName) : null;
|
||||
if (n == null) {
|
||||
if (create) {
|
||||
n = createNode(parentNode, mode, currentPath, currentName,true);
|
||||
n = createNode(parentNode, mode, currentPath, currentName,null, true);
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
|
@ -323,7 +327,7 @@ public class SimDistribStateManager implements DistribStateManager {
|
|||
return n;
|
||||
}
|
||||
|
||||
private Node createNode(Node parentNode, CreateMode mode, StringBuilder fullChildPath, String baseChildName, boolean attachToParent) throws IOException {
|
||||
private Node createNode(Node parentNode, CreateMode mode, StringBuilder fullChildPath, String baseChildName, byte[] data, boolean attachToParent) throws IOException {
|
||||
String nodeName = baseChildName;
|
||||
if ((parentNode.mode == CreateMode.EPHEMERAL || parentNode.mode == CreateMode.EPHEMERAL_SEQUENTIAL) &&
|
||||
(mode == CreateMode.EPHEMERAL || mode == CreateMode.EPHEMERAL_SEQUENTIAL)) {
|
||||
|
@ -335,7 +339,7 @@ public class SimDistribStateManager implements DistribStateManager {
|
|||
}
|
||||
|
||||
fullChildPath.append(nodeName);
|
||||
Node child = new Node(parentNode, nodeName, fullChildPath.toString(), mode, id);
|
||||
Node child = new Node(parentNode, nodeName, fullChildPath.toString(), data, mode, id);
|
||||
|
||||
if (attachToParent) {
|
||||
parentNode.setChild(nodeName, child);
|
||||
|
@ -480,13 +484,9 @@ public class SimDistribStateManager implements DistribStateManager {
|
|||
multiLock.lock();
|
||||
try {
|
||||
String nodeName = elements[elements.length-1];
|
||||
Node childNode = createNode(parentNode, mode, parentStringBuilder.append("/"), nodeName, false);
|
||||
childNode.setData(data, -1);
|
||||
Node childNode = createNode(parentNode, mode, parentStringBuilder.append("/"), nodeName, data,false);
|
||||
parentNode.setChild(childNode.name, childNode);
|
||||
return childNode.path;
|
||||
} catch (BadVersionException e) {
|
||||
// not happening
|
||||
return null;
|
||||
} finally {
|
||||
multiLock.unlock();
|
||||
}
|
||||
|
@ -586,7 +586,7 @@ public class SimDistribStateManager implements DistribStateManager {
|
|||
@Override
|
||||
public AutoScalingConfig getAutoScalingConfig(Watcher watcher) throws InterruptedException, IOException {
|
||||
Map<String, Object> map = new HashMap<>();
|
||||
int version = -1;
|
||||
int version = 0;
|
||||
try {
|
||||
VersionedData data = getData(ZkStateReader.SOLR_AUTOSCALING_CONF_PATH, watcher);
|
||||
if (data != null && data.getData() != null && data.getData().length > 0) {
|
||||
|
|
|
@ -341,6 +341,35 @@ public class TestSimDistribStateManager extends SolrTestCaseJ4 {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testNewlyCreatedPathsStartWithVersionZero() throws Exception {
|
||||
stateManager.makePath("/createdWithoutData");
|
||||
VersionedData vd = stateManager.getData("/createdWithoutData", null);
|
||||
assertEquals(0, vd.getVersion());
|
||||
|
||||
stateManager.createData("/createdWithData", new String("helloworld").getBytes(StandardCharsets.UTF_8), CreateMode.PERSISTENT);
|
||||
vd = stateManager.getData("/createdWithData");
|
||||
assertEquals(0, vd.getVersion());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testModifiedDataNodesGetUpdatedVersion() throws Exception {
|
||||
stateManager.createData("/createdWithData", new String("foo").getBytes(StandardCharsets.UTF_8), CreateMode.PERSISTENT);
|
||||
VersionedData vd = stateManager.getData("/createdWithData");
|
||||
assertEquals(0, vd.getVersion());
|
||||
|
||||
stateManager.setData("/createdWithData", new String("bar").getBytes(StandardCharsets.UTF_8), 0);
|
||||
vd = stateManager.getData("/createdWithData");
|
||||
assertEquals(1, vd.getVersion());
|
||||
}
|
||||
|
||||
// This is a little counterintuitive, so probably worth its own testcase so we don't break it accidentally.
|
||||
@Test
|
||||
public void testHasDataReturnsTrueForExistingButEmptyNodes() throws Exception {
|
||||
stateManager.makePath("/nodeWithoutData");
|
||||
assertTrue(stateManager.hasData("/nodeWithoutData"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testMulti() throws Exception {
|
||||
|
||||
|
|
|
@ -311,7 +311,7 @@ public class AutoScalingConfig implements MapWriter {
|
|||
*/
|
||||
public AutoScalingConfig(Map<String, Object> jsonMap) {
|
||||
this.jsonMap = jsonMap;
|
||||
int version = -1;
|
||||
int version = 0;
|
||||
if (jsonMap.containsKey(AutoScalingParams.ZK_VERSION)) {
|
||||
try {
|
||||
version = (Integer)jsonMap.get(AutoScalingParams.ZK_VERSION);
|
||||
|
|
Loading…
Reference in New Issue