Skip to content

Commit

Permalink
SOLR-16110: Using Schema/Config API breaks the File-Upload of Config …
Browse files Browse the repository at this point in the history 
…Set File (#831)

Co-authored-by: Steffen Moldenhauer <[email protected]>
  • Loading branch information
@risdenk @smoldenhauer-ish
2 people authored and Ishan Chattopadhyaya committed Oct 16, 2023
1 parent 89fb1ee commit 7bd8229
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 2 deletions.
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ Bug Fixes

* SOLR-16165: Rare Deadlock in SlotAcc initialization (Justin Sweeney, noble)

* SOLR-16110: Using Schema/Config API breaks the File-Upload of Config Set File (Steffen Moldenhauer, Kevin Risden)

Optimizations
---------------------
* SOLR-16555: SolrIndexSearcher - FilterCache intersections/andNot should not clone bitsets repeatedly (Kevin Risden, David Smiley)
Expand Down
10 changes: 8 additions & 2 deletions solr/core/src/java/org/apache/solr/cloud/ZkController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ public class ZkController implements Closeable {
public final static String COLLECTION_PARAM_PREFIX = "collection.";
public final static String CONFIGNAME_PROP = "configName";

public static final byte[] TOUCHED_ZNODE_DATA = "{}".getBytes(StandardCharsets.UTF_8);

static class ContextKey {

private String collection;
Expand Down Expand Up @@ -2503,13 +2505,17 @@ public static int persistConfigResourceToZooKeeper(ZkSolrResourceLoader zkLoader

public static void touchConfDir(ZkSolrResourceLoader zkLoader) {
SolrZkClient zkClient = zkLoader.getZkController().getZkClient();
String configSetZkPath = zkLoader.getConfigSetZkPath();
try {
zkClient.setData(zkLoader.getConfigSetZkPath(), new byte[]{0}, true);
// Ensure that version gets updated by replacing data with itself.
// If there is no existing data then set it to byte[] {0}.
// This should trigger any watchers if necessary as well.
zkClient.atomicUpdate(configSetZkPath, bytes -> bytes == null ? TOUCHED_ZNODE_DATA : bytes);
} catch (Exception e) {
if (e instanceof InterruptedException) {
Thread.currentThread().interrupt(); // Restore the interrupted status
}
final String msg = "Error 'touching' conf location " + zkLoader.getConfigSetZkPath();
final String msg = "Error 'touching' conf location " + configSetZkPath;
log.error(msg, e);
throw new SolrException(ErrorCode.SERVER_ERROR, msg, e);

Expand Down
58 changes: 58 additions & 0 deletions solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import java.util.Deque;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -71,8 +72,10 @@
import org.apache.solr.client.solrj.request.ConfigSetAdminRequest.Delete;
import org.apache.solr.client.solrj.request.ConfigSetAdminRequest.Upload;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.request.schema.SchemaRequest;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
import org.apache.solr.client.solrj.response.ConfigSetAdminResponse;
import org.apache.solr.client.solrj.response.schema.SchemaResponse;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.cloud.SolrZkClient;
Expand Down Expand Up @@ -121,6 +124,7 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {

@BeforeClass
public static void setUpClass() throws Exception {
System.setProperty("managed.schema.mutable", "true");
configureCluster(1)
.withSecurityJson(getSecurityJson())
.configure();
Expand All @@ -130,6 +134,7 @@ public static void setUpClass() throws Exception {
@AfterClass
public static void tearDownClass() throws Exception {
zkConfigManager = null;
System.clearProperty("managed.schema.mutable");
}

@Override
Expand Down Expand Up @@ -613,6 +618,59 @@ public void testNewSingleFile(boolean v2) throws Exception {
}
}

@Test
public void testNewSingleFileAfterSchemaAPIV1() throws Exception {
testNewSingleFileAfterSchemaAPI(false);
}

@Test
public void testNewSingleFileAfterSchemaAPIV2() throws Exception {
testNewSingleFileAfterSchemaAPI(true);
}

private void addStringField(String fieldName, String collection, CloudSolrClient cloudClient)
throws IOException, SolrServerException {
Map<String, Object> fieldAttributes = new LinkedHashMap<>();
fieldAttributes.put("name", fieldName);
fieldAttributes.put("type", "string");
SchemaRequest.AddField addFieldUpdateSchemaRequest =
new SchemaRequest.AddField(fieldAttributes);
SchemaResponse.UpdateResponse addFieldResponse =
addFieldUpdateSchemaRequest.process(cloudClient, collection);
assertEquals(0, addFieldResponse.getStatus());
assertNull(addFieldResponse.getResponse().get("errors"));

log.info("added new field={}", fieldName);
}

private void testNewSingleFileAfterSchemaAPI(boolean v2) throws Exception {
String collectionName = "newcollection";
String configsetName = "regular";
String configsetSuffix = "testSinglePathNew-1-" + v2;
createConfigSet(null, configsetName + configsetSuffix, null, cluster.getSolrClient(), "solr");
createCollection(
collectionName, configsetName + configsetSuffix, 1, 1, cluster.getSolrClient());
addStringField("newField", collectionName, cluster.getSolrClient());

assertEquals(
0,
uploadSingleConfigSetFile(
configsetName,
configsetSuffix,
"solr",
"solr/configsets/upload/regular/solrconfig.xml",
"/test/upload/path/solrconfig.xml",
false,
false,
v2));
SolrZkClient zkClient = cluster.getZkServer().getZkClient();
assertEquals(
"Expecting first version of new file",
0,
getConfigZNodeVersion(
zkClient, configsetName, configsetSuffix, "test/upload/path/solrconfig.xml"));
}

@Test
public void testSingleWithCleanupV1() throws Exception {
testSingleWithCleanup(false);
Expand Down
65 changes: 65 additions & 0 deletions solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.solr.cloud;

import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -43,6 +44,7 @@
import org.apache.solr.update.UpdateShardHandlerConfig;
import org.apache.solr.util.LogLevel;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.data.Stat;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
Expand Down Expand Up @@ -349,6 +351,69 @@ public List<CoreDescriptor> getCurrentDescriptors() {
}
}

@Test
public void testTouchConfDir() throws Exception {
Path zkDir = createTempDir("zkData");
ZkTestServer server = new ZkTestServer(zkDir);
try {
server.run();
try (SolrZkClient zkClient = new SolrZkClient(server.getZkAddress(), TIMEOUT)) {
CoreContainer cc = getCoreContainer();
try {
CloudConfig cloudConfig =
new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983, "solr").build();
try (ZkController zkController = new ZkController(cc, server.getZkAddress(), TIMEOUT, cloudConfig, new CurrentCoreDescriptorProvider() {

@Override
public List<CoreDescriptor> getCurrentDescriptors() {
// do nothing
return null;
}
})) {
final Path dir = createTempDir();
final String configsetName = "testconfigset";
try (ZkSolrResourceLoader loader =
new ZkSolrResourceLoader(dir, configsetName, null, new Properties(), zkController)) {
String zkpath = "/configs/" + configsetName;

// touchConfDir doesn't make the znode
Stat s = new Stat();
assertFalse(zkClient.exists(zkpath, true));
zkClient.makePath(zkpath, true);
assertTrue(zkClient.exists(zkpath, true));
assertNull(zkClient.getData(zkpath, null, s, true));
assertEquals(0, s.getVersion());

// touchConfDir should only set the data to new byte[] {0}
ZkController.touchConfDir(loader);
assertTrue(zkClient.exists(zkpath, true));
assertArrayEquals(
ZkController.TOUCHED_ZNODE_DATA, zkClient.getData(zkpath, null, s, true));
assertEquals(1, s.getVersion());

// set new data to check if touchConfDir overwrites later
byte[] data = "{\"key\", \"new data\"".getBytes(StandardCharsets.UTF_8);
s = zkClient.setData(zkpath, data, true);
assertEquals(2, s.getVersion());

// make sure touchConfDir doesn't overwrite existing data.
// touchConfDir should update version.
assertTrue(zkClient.exists(zkpath, true));
ZkController.touchConfDir(loader);
assertTrue(zkClient.exists(zkpath, true));
assertArrayEquals(data, zkClient.getData(zkpath, null, s, true));
assertEquals(3, s.getVersion());
}
}
} finally {
cc.shutdown();
}
}
} finally {
server.shutdown();
}
}

private CoreContainer getCoreContainer() {
return new MockCoreContainer();
}
Expand Down
6 changes: 6 additions & 0 deletions solr/core/src/test/org/apache/solr/schema/TestManagedSchemaAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.apache.solr.client.solrj.response.schema.SchemaResponse;
import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.SolrInputDocument;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
import org.slf4j.Logger;
Expand All @@ -47,6 +48,11 @@ public static void createCluster() throws Exception {
.configure();
}

@AfterClass
public static void tearDownClass() {
System.clearProperty("managed.schema.mutable");
}

@Test
public void test() throws Exception {
String collection = "testschemaapi";
Expand Down

0 comments on commit 7bd8229

Please sign in to comment.