You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ep...@apache.org on 2024/02/13 13:49:10 UTC
(solr) branch branch_9x updated: SOLR-17148: Fixing Config API overlay property enabling or disabling cache (#2247)
This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new 3f9fcf81806 SOLR-17148: Fixing Config API overlay property enabling or disabling cache (#2247)
3f9fcf81806 is described below
commit 3f9fcf818066267ff5ad82a53f359660ee4dea5e
Author: Sanjay Dutt <sa...@gmail.com>
AuthorDate: Tue Feb 13 19:17:33 2024 +0530
SOLR-17148: Fixing Config API overlay property enabling or disabling cache (#2247)
---------
Co-authored-by: iamsanjay <sa...@yahoo.com>
---
solr/CHANGES.txt | 2 +
.../java/org/apache/solr/search/CacheConfig.java | 14 +-
.../test/org/apache/solr/core/CacheConfigTest.java | 168 +++++++++++++++++++++
.../apache/solr/core/TestSolrConfigHandler.java | 54 +++++++
4 files changed, 230 insertions(+), 8 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3eb685a9b85..ab43eb599a7 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -37,6 +37,8 @@ Bug Fixes
---------------------
* SOLR-17152: Better alignment of Admin UI graph (janhoy)
+* SOLR-17148: Fixing Config API overlay property enabling or disabling the cache (Sanjay Dutt, hossman, Eric Pugh)
+
Dependency Upgrades
---------------------
(No changes)
diff --git a/solr/core/src/java/org/apache/solr/search/CacheConfig.java b/solr/core/src/java/org/apache/solr/search/CacheConfig.java
index aa4d61a2204..83ce72af836 100644
--- a/solr/core/src/java/org/apache/solr/search/CacheConfig.java
+++ b/solr/core/src/java/org/apache/solr/search/CacheConfig.java
@@ -28,7 +28,6 @@ import java.util.function.Supplier;
import org.apache.solr.common.ConfigNode;
import org.apache.solr.common.MapSerializable;
import org.apache.solr.common.util.CollectionUtil;
-import org.apache.solr.common.util.StrUtils;
import org.apache.solr.core.PluginInfo;
import org.apache.solr.core.SolrConfig;
import org.apache.solr.core.SolrResourceLoader;
@@ -81,7 +80,9 @@ public class CacheConfig implements MapSerializable {
public static Map<String, CacheConfig> getMultipleConfigs(
SolrResourceLoader loader, SolrConfig solrConfig, String configPath, List<ConfigNode> nodes) {
- if (nodes == null || nodes.size() == 0) return new LinkedHashMap<>();
+ if (nodes == null || nodes.isEmpty()) {
+ return new LinkedHashMap<>();
+ }
Map<String, CacheConfig> result = CollectionUtil.newHashMap(nodes.size());
for (ConfigNode node : nodes) {
if (node.boolAttr("enabled", true)) {
@@ -94,11 +95,8 @@ public class CacheConfig implements MapSerializable {
}
public static CacheConfig getConfig(SolrConfig solrConfig, ConfigNode node, String xpath) {
- if (!node.exists() || !"true".equals(node.attributes().get("enabled", "true"))) {
- Map<String, Object> m = solrConfig.getOverlay().getEditableSubProperties(xpath);
- if (m == null) return null;
- List<String> parts = StrUtils.splitSmart(xpath, '/');
- return getConfig(solrConfig, parts.get(parts.size() - 1), Collections.emptyMap(), xpath);
+ if (!node.boolAttr("enabled", true) || !node.exists()) {
+ return null;
}
return getConfig(solrConfig, node.name(), node.attributes().asMap(), xpath);
}
@@ -178,7 +176,7 @@ public class CacheConfig implements MapSerializable {
}
@Override
- public Map<String, Object> toMap(Map<String, Object> map) {
+ public Map<String, Object> toMap(Map<String, Object> argsMap) {
// TODO: Should not create new HashMap?
return new HashMap<>(args);
}
diff --git a/solr/core/src/test/org/apache/solr/core/CacheConfigTest.java b/solr/core/src/test/org/apache/solr/core/CacheConfigTest.java
new file mode 100644
index 00000000000..f8825a7f108
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/core/CacheConfigTest.java
@@ -0,0 +1,168 @@
+/*
+ * 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.core;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.ConfigNode;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.search.CacheConfig;
+import org.apache.solr.util.DOMConfigNode;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.mockito.Mockito;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+
+/** Test for {@link CacheConfig} */
+public class CacheConfigTest extends SolrTestCaseJ4 {
+ private static final String XPATH_DOCUMENT_CACHE = "query/documentCache";
+ private static final String XPATH_QUERY_RESULT_CACHE = "query/queryResultCache";
+ private SolrResourceLoader mockSolrResourceLoader;
+ private ConfigOverlay mockConfigOverlay;
+ private ConfigNode overlayConfigNode;
+ private ConfigNode domConfigNode;
+ private ConfigNode domConfigNodeDisable;
+ private SolrConfig mockSolrConfig;
+ private static final DocumentBuilder docBuilder;
+
+ static {
+ try {
+ docBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
+ } catch (ParserConfigurationException e) {
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ }
+ }
+
+ @BeforeClass
+ public static void ensureWorkingMockito() {
+ SolrTestCaseJ4.assumeWorkingMockito();
+ }
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ super.setUp();
+ resetMocks();
+ final String initialSize = "99";
+ final String size = "999";
+
+ final Document doc = docBuilder.newDocument();
+ final Element documentCacheNode = doc.createElement("documentCache");
+ documentCacheNode.setAttribute("initialSize", initialSize);
+ documentCacheNode.setAttribute("size", size);
+ documentCacheNode.setAttribute("enabled", "true");
+ domConfigNode = new DOMConfigNode(documentCacheNode);
+ Mockito.when(mockSolrConfig.getOverlay()).thenReturn(mockConfigOverlay);
+ Mockito.when(mockConfigOverlay.getEditableSubProperties(XPATH_DOCUMENT_CACHE)).thenReturn(null);
+
+ final String sizeForQueryResCache = "199";
+ final Document queryResultCacheDoc = docBuilder.newDocument();
+ final Element queryResultNode = queryResultCacheDoc.createElement("queryResultCache");
+ queryResultNode.setAttribute("initialSize", "99");
+ queryResultNode.setAttribute("size", sizeForQueryResCache);
+ queryResultNode.setAttribute("enabled", "false");
+ domConfigNodeDisable = new DOMConfigNode(queryResultNode);
+ }
+
+ public void testSolrConfigPropsForCache() {
+ final CacheConfig cacheConfig =
+ CacheConfig.getConfig(mockSolrConfig, domConfigNode, XPATH_DOCUMENT_CACHE);
+ assertNotNull(cacheConfig);
+ final Map<String, Object> args = cacheConfig.toMap(new HashMap<>());
+ assertNotNull(args);
+ assertEquals("99", args.get("initialSize"));
+ assertEquals("999", args.get("size"));
+ assertEquals("documentCache", cacheConfig.getNodeName());
+ }
+
+ public void testSolrConfigDisabledCache() {
+ Mockito.when(mockSolrConfig.getOverlay()).thenReturn(mockConfigOverlay);
+ Mockito.when(mockConfigOverlay.getEditableSubProperties(XPATH_QUERY_RESULT_CACHE))
+ .thenReturn(null);
+
+ final CacheConfig cacheConfig =
+ CacheConfig.getConfig(mockSolrConfig, domConfigNodeDisable, XPATH_QUERY_RESULT_CACHE);
+ assertNull(cacheConfig);
+ }
+
+ public void testOverlayPropsOverridingSolrConfigProps() {
+ final String overlaidSize = "199";
+ overlayConfigNode =
+ new OverlaidConfigNode(mockConfigOverlay, "documentCache", null, domConfigNode);
+ Mockito.when(mockSolrConfig.getOverlay()).thenReturn(mockConfigOverlay);
+ Mockito.when(mockConfigOverlay.getEditableSubProperties(XPATH_DOCUMENT_CACHE))
+ .thenReturn(Map.of("size", overlaidSize));
+
+ final CacheConfig cacheConfig =
+ CacheConfig.getConfig(mockSolrConfig, domConfigNode, XPATH_DOCUMENT_CACHE);
+ assertNotNull(cacheConfig);
+ final Map<String, Object> args = cacheConfig.toMap(new HashMap<>());
+ assertNotNull(args);
+ assertEquals(overlaidSize, args.get("size"));
+ }
+
+ public void testOverlaidDisabledSolrConfigEnabledCache() {
+ overlayConfigNode =
+ new OverlaidConfigNode(mockConfigOverlay, "queryResultCache", null, domConfigNode);
+ Mockito.when(mockSolrConfig.getOverlay()).thenReturn(mockConfigOverlay);
+ Mockito.when(mockConfigOverlay.getXPathProperty(Arrays.asList("queryResultCache", "enabled")))
+ .thenReturn("false");
+
+ final CacheConfig cacheConfig =
+ CacheConfig.getConfig(mockSolrConfig, overlayConfigNode, XPATH_QUERY_RESULT_CACHE);
+ assertNull(cacheConfig);
+ }
+
+ public void testOverlaidEnabledSolrConfigDisabledCache() {
+ overlayConfigNode =
+ new OverlaidConfigNode(mockConfigOverlay, "queryResultCache", null, domConfigNodeDisable);
+ Mockito.when(mockSolrConfig.getOverlay()).thenReturn(mockConfigOverlay);
+ Mockito.when(mockConfigOverlay.getXPathProperty(Arrays.asList("queryResultCache", "enabled")))
+ .thenReturn("true");
+
+ final CacheConfig cacheConfig =
+ CacheConfig.getConfig(mockSolrConfig, overlayConfigNode, XPATH_QUERY_RESULT_CACHE);
+ assertNotNull(cacheConfig);
+ final Map<String, Object> args = cacheConfig.toMap(new HashMap<>());
+ assertNotNull(args);
+ assertEquals("99", args.get("initialSize"));
+ assertEquals("queryResultCache", cacheConfig.getNodeName());
+ }
+
+ public void testEmptyConfigNodeCache() {
+ final ConfigNode emptyNodeSolrConfig = ConfigNode.EMPTY;
+ final CacheConfig cacheConfig =
+ CacheConfig.getConfig(mockSolrConfig, emptyNodeSolrConfig, null);
+ assertNull(cacheConfig);
+ }
+
+ private void resetMocks() {
+ mockSolrConfig = mock(SolrConfig.class);
+ mockConfigOverlay = mock(ConfigOverlay.class);
+ mockSolrResourceLoader = mock(SolrResourceLoader.class);
+ when(mockSolrConfig.getResourceLoader()).thenReturn(mockSolrResourceLoader);
+ }
+}
diff --git a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
index 515f28c86eb..9ef6256d991 100644
--- a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
+++ b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java
@@ -1011,4 +1011,58 @@ public class TestSolrConfigHandler extends RestTestBase {
return new LinkedHashMapWriter<>();
}
}
+
+ public void testCacheDisableSolrConfig() throws Exception {
+ RESTfulServerProvider oldProvider = restTestHarness.getServerProvider();
+ restTestHarness.setServerProvider(() -> getBaseUrl());
+ MapWriter confMap = getRespMap("/admin/metrics", restTestHarness);
+ assertNotNull(
+ confMap._get(
+ asList("metrics", "solr.core.collection1", "CACHE.searcher.fieldValueCache"), null));
+ // Here documentCache is disabled at initialization in SolrConfig
+ assertNull(
+ confMap._get(
+ asList("metrics", "solr.core.collection1", "CACHE.searcher.documentCache"), null));
+ restTestHarness.setServerProvider(oldProvider);
+ }
+
+ public void testSetPropertyCacheSize() throws Exception {
+ RESTfulServerProvider oldProvider = restTestHarness.getServerProvider();
+ // Changing cache size
+ String payload = "{'set-property' : { 'query.documentCache.size': 399} }";
+ runConfigCommand(restTestHarness, "/config", payload);
+ MapWriter overlay = getRespMap("/config/overlay", restTestHarness);
+ assertEquals("399", overlay._getStr("overlay/props/query/documentCache/size", null));
+ // Setting size only will not enable the cache
+ restTestHarness.setServerProvider(() -> getBaseUrl());
+ MapWriter confMap = getRespMap("/admin/metrics", restTestHarness);
+ assertNull(
+ confMap._get(
+ asList("metrics", "solr.core.collection1", "CACHE.searcher.documentCache"), null));
+ restTestHarness.setServerProvider(oldProvider);
+ }
+
+ public void testSetPropertyEnableAndDisableCache() throws Exception {
+ RESTfulServerProvider oldProvider = restTestHarness.getServerProvider();
+ // Enabling Cache
+ String payload = "{'set-property' : { 'query.documentCache.enabled': true} }";
+ runConfigCommand(restTestHarness, "/config", payload);
+ restTestHarness.setServerProvider(() -> getBaseUrl());
+ MapWriter confMap = getRespMap("/admin/metrics", restTestHarness);
+ assertNotNull(
+ confMap._get(
+ asList("metrics", "solr.core.collection1", "CACHE.searcher.documentCache"), null));
+
+ // Disabling Cache
+ payload = "{ 'set-property' : { 'query.documentCache.enabled': false } }";
+ restTestHarness.setServerProvider(oldProvider);
+ runConfigCommand(restTestHarness, "/config", payload);
+ restTestHarness.setServerProvider(() -> getBaseUrl());
+ confMap = getRespMap("/admin/metrics", restTestHarness);
+ assertNull(
+ confMap._get(
+ asList("metrics", "solr.core.collection1", "CACHE.searcher.documentCache"), null));
+
+ restTestHarness.setServerProvider(oldProvider);
+ }
}