asalamon74 commented on a change in pull request #462: URL: https://github.com/apache/solr/pull/462#discussion_r795691216
########## File path: solr/core/src/test-files/solr/configsets/cloud-managed-autocommit/conf/solrconfig.xml ########## @@ -0,0 +1,61 @@ +<?xml version="1.0" ?> + +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<!-- Minimal solrconfig.xml with /select, /admin and /update only --> + +<config> + + <dataDir>${solr.data.dir:}</dataDir> + + <directoryFactory name="DirectoryFactory" + class="${solr.directoryFactory:solr.NRTCachingDirectoryFactory}"/> + + <schemaFactory class="ManagedIndexSchemaFactory"> + <bool name="mutable">${managed.schema.mutable}</bool> + <str name="managedSchemaResourceName">managed-schema</str> + </schemaFactory> + + <luceneMatchVersion>${tests.luceneMatchVersion:LATEST}</luceneMatchVersion> + + <updateHandler class="solr.DirectUpdateHandler2"> + <commitWithin> + <softCommit>${solr.commitwithin.softcommit:true}</softCommit> + </commitWithin> + <updateLog class="${solr.ulog:solr.UpdateLog}"></updateLog> + + <autoCommit> + <maxTime>${solr.autoCommit.maxTime:15000}</maxTime> + <openSearcher>false</openSearcher> + </autoCommit> + + <autoSoftCommit> + <maxTime>${solr.autoSoftCommit.maxTime:-1}</maxTime> + </autoSoftCommit> + </updateHandler> + + <requestHandler name="/select" class="solr.SearchHandler"> + <lst name="defaults"> + <str name="echoParams">explicit</str> + <str name="indent">true</str> + <str name="df">text</str> + </lst> + + </requestHandler> +</config> + Review comment: When I apply the patch I got the following warning, I think we don't need the empty line here. ``` warning: 1 line adds whitespace errors. ``` ########## File path: solr/core/src/test/org/apache/solr/schema/TestManagedSchemaWithMultipleAdd.java ########## @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.schema; + +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.client.solrj.request.schema.SchemaRequest; +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; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.util.LinkedHashMap; +import java.util.Map; + +public class TestManagedSchemaWithMultipleAdd extends SolrCloudTestCase { + + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + @BeforeClass + public static void createClusterAndInitSysProperties() throws Exception { + System.setProperty("managed.schema.mutable", "true"); + System.setProperty("solr.autoCommit.maxTime", "10000"); Review comment: I think we don't need to specify the hard commit time here. ########## File path: solr/core/src/test/org/apache/solr/schema/TestManagedSchemaWithMultipleAdd.java ########## @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.schema; + +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.client.solrj.request.schema.SchemaRequest; +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; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.util.LinkedHashMap; +import java.util.Map; + +public class TestManagedSchemaWithMultipleAdd extends SolrCloudTestCase { + + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + @BeforeClass + public static void createClusterAndInitSysProperties() throws Exception { + System.setProperty("managed.schema.mutable", "true"); + System.setProperty("solr.autoCommit.maxTime", "10000"); + System.setProperty("solr.autoSoftCommit.maxTime", "3000"); + configureCluster(1) + .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-managed-autocommit").resolve("conf")) + .configure(); + } + + @AfterClass + public static void afterRestartWhileUpdatingTest() { + System.clearProperty("managed.schema.mutable"); + System.clearProperty("solr.autoCommit.maxTime"); + System.clearProperty("solr.autoSoftCommit.maxTime"); + } + + @Test + public void test() throws Exception { + String collection = "testschemaapi"; + CollectionAdminRequest.createCollection(collection, "conf1", 1, 1) + .process(cluster.getSolrClient()); + testAddFieldAndMultipleDocument(collection); + } + + private void testAddFieldAndMultipleDocument(String collection) throws IOException, SolrServerException, InterruptedException { + + CloudSolrClient cloudClient = cluster.getSolrClient(); + + String fieldName = "myNewField1"; + addStringField(fieldName, collection, cloudClient); + + UpdateRequest ureq = new UpdateRequest(); + int numDocs = 1000; + for (int i=0;i<numDocs;i++) { + SolrInputDocument doc = new SolrInputDocument(); + doc.addField("id", i); + doc.addField(fieldName, "value" + i); + ureq = ureq.add(doc); + } + cloudClient.request(ureq, collection); + + // Wait for autoCommit to finish if there is one. Review comment: Usually it's better to call an explicit commit in the unit test to avoid timing problems but based on your jira comment manual commit works correctly: > If I commit manually (for example with curl) all the documents become available. You could expand the comment to show that we really need autocommit here. ########## File path: solr/core/src/java/org/apache/solr/schema/SchemaManager.java ########## @@ -126,11 +127,16 @@ public SchemaManager(SolrQueryRequest req){ } try { - latestVersion = ZkController.persistConfigResourceToZooKeeper - (zkLoader, managedIndexSchema.getSchemaZkVersion(), managedIndexSchema.getResourceName(), - sw.toString().getBytes(StandardCharsets.UTF_8), true); - req.getCore().getCoreContainer().reload(req.getCore().getName(), req.getCore().uniqueId); - break; + SolrConfigHandler configHandler = ((SolrConfigHandler)core.getRequestHandler("/config")); Review comment: We use `core` in the locking but `req.getCore()` later. It seems to me this represents the same core, but it would be better to use the same variable here. Maybe using `req.getCore()` is the easier change. ########## File path: solr/core/src/test/org/apache/solr/schema/TestManagedSchemaWithMultipleAdd.java ########## @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.schema; + +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.UpdateRequest; +import org.apache.solr.client.solrj.request.schema.SchemaRequest; +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; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.util.LinkedHashMap; +import java.util.Map; + +public class TestManagedSchemaWithMultipleAdd extends SolrCloudTestCase { + + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + @BeforeClass + public static void createClusterAndInitSysProperties() throws Exception { + System.setProperty("managed.schema.mutable", "true"); + System.setProperty("solr.autoCommit.maxTime", "10000"); + System.setProperty("solr.autoSoftCommit.maxTime", "3000"); + configureCluster(1) + .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-managed-autocommit").resolve("conf")) + .configure(); + } + + @AfterClass + public static void afterRestartWhileUpdatingTest() { + System.clearProperty("managed.schema.mutable"); + System.clearProperty("solr.autoCommit.maxTime"); Review comment: If we don't specify the value in `createClusterAndInitSysProperties()` we don't need to clean it here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
