From 3816a0eb2bbd1929523ae27db3c90d0942ed5f5f Mon Sep 17 00:00:00 2001 From: Mike McCandless Date: Wed, 10 Aug 2016 05:08:30 -0400 Subject: [PATCH 01/48] add comment --- lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java | 1 + 1 file changed, 1 insertion(+) diff --git a/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java b/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java index 7ad208ff22f..44c04e54b7c 100644 --- a/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java +++ b/lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java @@ -405,6 +405,7 @@ public class LRUQueryCache implements QueryCache, Accountable { lock.lock(); try { cache.clear(); + // Note that this also clears the uniqueQueries map since mostRecentlyUsedQueries is the uniqueQueries.keySet view: mostRecentlyUsedQueries.clear(); onClear(); } finally { From dd03d39dd6624a5d41325397ca246e01b12ec71d Mon Sep 17 00:00:00 2001 From: Alexandre Rafalovitch Date: Wed, 10 Aug 2016 21:26:50 +1000 Subject: [PATCH 02/48] SOLR-9003: DIH Debug now works in new Admin UI --- solr/CHANGES.txt | 2 ++ .../web/js/angular/controllers/dataimport.js | 18 ++++++++++++++---- solr/webapp/web/partials/dataimport.html | 8 ++++---- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 68834576ad4..3451a4f96c5 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -190,6 +190,8 @@ Bug Fixes * SOLR-8379: UI Cloud->Tree view now shows .txt files correctly (Alexandre Rafalovitch via janhoy) +* SOLR-9003: New Admin UI's Dataimport screen now correctly displays DIH Debug output (Alexandre Rafalovitch) + * SOLR-9308: Fix distributed RTG to forward request params, fixes fq and non-default fl params (hossman) * SOLR-9179: NPE in IndexSchema using IBM JDK (noble, Colvin Cowie) diff --git a/solr/webapp/web/js/angular/controllers/dataimport.js b/solr/webapp/web/js/angular/controllers/dataimport.js index 9ca723985da..a051ad2178b 100644 --- a/solr/webapp/web/js/angular/controllers/dataimport.js +++ b/solr/webapp/web/js/angular/controllers/dataimport.js @@ -61,6 +61,10 @@ solrAdminApp.controller('DataImportController', $scope.toggleDebug = function () { $scope.isDebugMode = !$scope.isDebugMode; + if ($scope.isDebugMode) { + // also enable Debug checkbox + $scope.form.showDebug = true; + } $scope.showConfiguration = true; } @@ -100,7 +104,13 @@ solrAdminApp.controller('DataImportController', $scope.submit = function () { var params = {}; for (var key in $scope.form) { - params[key] = $scope.form[key]; + if (key == "showDebug") { + if ($scope.form.showDebug) { + params["debug"] = true; + } + } else { + params[key] = $scope.form[key]; + } } if (params.custom.length) { var customParams = $scope.form.custom.split("&"); @@ -111,10 +121,10 @@ solrAdminApp.controller('DataImportController', } delete params.custom; - if (params.isDebugMode) { - params.dataConfig = $scope.rawConfig; + if ($scope.isDebugMode) { + params.dataConfig = $scope.config; } - delete params.showDebug; + params.core = $routeParams.core; DataImport.post(params, function (data) { diff --git a/solr/webapp/web/partials/dataimport.html b/solr/webapp/web/partials/dataimport.html index 5fde5975eaa..5cd6d2c3a0b 100644 --- a/solr/webapp/web/partials/dataimport.html +++ b/solr/webapp/web/partials/dataimport.html @@ -94,7 +94,7 @@ limitations under the License.
- +
@@ -115,11 +115,11 @@ limitations under the License.
- + No Request executed - -
+ +
From 64c99293d7d73c798c794cc647cf19636f62b2d6 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Wed, 10 Aug 2016 20:23:22 +0530 Subject: [PATCH 03/48] SOLR-9397: Config API does not support adding caches --- solr/CHANGES.txt | 2 + .../java/org/apache/solr/core/SolrConfig.java | 29 ++++++-- .../org/apache/solr/search/CacheConfig.java | 24 ++++--- .../apache/solr/search/SolrIndexSearcher.java | 69 +++---------------- .../solr/core/TestSolrConfigHandler.java | 56 ++++++++++++++- 5 files changed, 106 insertions(+), 74 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 3451a4f96c5..05252c33469 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -196,6 +196,8 @@ Bug Fixes * SOLR-9179: NPE in IndexSchema using IBM JDK (noble, Colvin Cowie) +* SOLR-9397: Config API does not support adding caches (noble) + Optimizations ---------------------- diff --git a/solr/core/src/java/org/apache/solr/core/SolrConfig.java b/solr/core/src/java/org/apache/solr/core/SolrConfig.java index eb3aa5fc7f1..653c612fe65 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrConfig.java +++ b/solr/core/src/java/org/apache/solr/core/SolrConfig.java @@ -28,7 +28,17 @@ import java.net.URL; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.UUID; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -49,6 +59,7 @@ import org.apache.solr.schema.IndexSchemaFactory; import org.apache.solr.search.CacheConfig; import org.apache.solr.search.FastLRUCache; import org.apache.solr.search.QParserPlugin; +import org.apache.solr.search.SolrCache; import org.apache.solr.search.ValueSourceParser; import org.apache.solr.search.stats.StatsCache; import org.apache.solr.servlet.SolrRequestParsers; @@ -91,7 +102,7 @@ public class SolrConfig extends Config implements MapSerializable { public static final String DEFAULT_CONF_FILE = "solrconfig.xml"; private RequestParams requestParams; - public static enum PluginOpts { + public enum PluginOpts { MULTI_OK, REQUIRE_NAME, REQUIRE_NAME_IN_OVERLAY, @@ -254,7 +265,6 @@ public class SolrConfig extends Config implements MapSerializable { dataDir = get("dataDir", null); if (dataDir != null && dataDir.length() == 0) dataDir = null; - userCacheConfigs = CacheConfig.getMultipleConfigs(this, "query/cache"); org.apache.solr.search.SolrIndexSearcher.initRegenerators(this); @@ -276,6 +286,16 @@ public class SolrConfig extends Config implements MapSerializable { maxWarmingSearchers = getInt("query/maxWarmingSearchers", Integer.MAX_VALUE); slowQueryThresholdMillis = getInt("query/slowQueryThresholdMillis", -1); for (SolrPluginInfo plugin : plugins) loadPluginInfo(plugin); + + Map userCacheConfigs = CacheConfig.getMultipleConfigs(this, "query/cache"); + List caches = getPluginInfos(SolrCache.class.getName()); + if (!caches.isEmpty()) { + for (PluginInfo c : caches) { + userCacheConfigs.put(c.name, CacheConfig.getConfig(this, "cache", c.attributes, null)); + } + } + this.userCacheConfigs = Collections.unmodifiableMap(userCacheConfigs); + updateHandlerInfo = loadUpdatehandlerInfo(); multipartUploadLimitKB = getInt( @@ -317,6 +337,7 @@ public class SolrConfig extends Config implements MapSerializable { .add(new SolrPluginInfo(TransformerFactory.class, "transformer", REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK)) .add(new SolrPluginInfo(SearchComponent.class, "searchComponent", REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK)) .add(new SolrPluginInfo(UpdateRequestProcessorFactory.class, "updateProcessor", REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK)) + .add(new SolrPluginInfo(SolrCache.class, "cache", REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK)) // TODO: WTF is up with queryConverter??? // it apparently *only* works as a singleton? - SOLR-4304 // and even then -- only if there is a single SpellCheckComponent @@ -457,7 +478,7 @@ public class SolrConfig extends Config implements MapSerializable { public final CacheConfig queryResultCacheConfig; public final CacheConfig documentCacheConfig; public final CacheConfig fieldValueCacheConfig; - public final CacheConfig[] userCacheConfigs; + public final Map userCacheConfigs; // SolrIndexSearcher - more... public final boolean useFilterForSortedQuery; public final int queryResultWindowSize; 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 40e54dccb4e..ee333f8787b 100644 --- a/solr/core/src/java/org/apache/solr/search/CacheConfig.java +++ b/solr/core/src/java/org/apache/solr/search/CacheConfig.java @@ -17,10 +17,10 @@ package org.apache.solr.search; import javax.xml.xpath.XPathConstants; - import java.lang.invoke.MethodHandles; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -75,14 +75,15 @@ public class CacheConfig implements MapSerializable{ this.regenerator = regenerator; } - public static CacheConfig[] getMultipleConfigs(SolrConfig solrConfig, String configPath) { - NodeList nodes = (NodeList)solrConfig.evaluate(configPath, XPathConstants.NODESET); - if (nodes==null || nodes.getLength()==0) return null; - CacheConfig[] configs = new CacheConfig[nodes.getLength()]; - for (int i=0; i getMultipleConfigs(SolrConfig solrConfig, String configPath) { + NodeList nodes = (NodeList) solrConfig.evaluate(configPath, XPathConstants.NODESET); + if (nodes == null || nodes.getLength() == 0) return new LinkedHashMap<>(); + Map result = new HashMap<>(nodes.getLength()); + for (int i = 0; i < nodes.getLength(); i++) { + CacheConfig config = getConfig(solrConfig, nodes.item(i).getNodeName(), DOMUtil.toMap(nodes.item(i).getAttributes()), configPath); + result.put(config.args.get(NAME), config); } - return configs; + return result; } @@ -101,9 +102,14 @@ public class CacheConfig implements MapSerializable{ public static CacheConfig getConfig(SolrConfig solrConfig, String nodeName, Map attrs, String xpath) { CacheConfig config = new CacheConfig(); config.nodeName = nodeName; + Map attrsCopy = new LinkedHashMap<>(attrs.size()); + for (Map.Entry e : attrs.entrySet()) { + attrsCopy.put(e.getKey(), String.valueOf(e.getValue())); + } + attrs = attrsCopy; config.args = attrs; - Map map = solrConfig.getOverlay().getEditableSubProperties(xpath); + Map map = xpath == null ? null : solrConfig.getOverlay().getEditableSubProperties(xpath); if(map != null){ HashMap mapCopy = new HashMap<>(config.args); for (Map.Entry e : map.entrySet()) { diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java index cc719f0be37..0f480c67579 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java +++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java @@ -36,62 +36,16 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import com.google.common.base.Function; +import com.google.common.base.Objects; +import com.google.common.collect.Iterables; import org.apache.lucene.document.Document; import org.apache.lucene.document.DocumentStoredFieldVisitor; import org.apache.lucene.document.LazyDocument; -import org.apache.lucene.index.BinaryDocValues; -import org.apache.lucene.index.DirectoryReader; -import org.apache.lucene.index.DocValues; -import org.apache.lucene.index.DocValuesType; -import org.apache.lucene.index.ExitableDirectoryReader; -import org.apache.lucene.index.FieldInfo; -import org.apache.lucene.index.FieldInfos; -import org.apache.lucene.index.IndexReader; -import org.apache.lucene.index.IndexableField; -import org.apache.lucene.index.LeafReader; -import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.MultiPostingsEnum; -import org.apache.lucene.index.NumericDocValues; -import org.apache.lucene.index.PostingsEnum; -import org.apache.lucene.index.SortedDocValues; -import org.apache.lucene.index.SortedSetDocValues; -import org.apache.lucene.index.StoredFieldVisitor; +import org.apache.lucene.index.*; import org.apache.lucene.index.StoredFieldVisitor.Status; -import org.apache.lucene.index.Term; -import org.apache.lucene.index.TermContext; -import org.apache.lucene.index.Terms; -import org.apache.lucene.index.TermsEnum; -import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.*; import org.apache.lucene.search.BooleanClause.Occur; -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.CollectionStatistics; -import org.apache.lucene.search.Collector; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.DocIdSet; -import org.apache.lucene.search.DocIdSetIterator; -import org.apache.lucene.search.EarlyTerminatingSortingCollector; -import org.apache.lucene.search.Explanation; -import org.apache.lucene.search.FieldDoc; -import org.apache.lucene.search.IndexSearcher; -import org.apache.lucene.search.LeafCollector; -import org.apache.lucene.search.MatchAllDocsQuery; -import org.apache.lucene.search.MultiCollector; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.ScoreDoc; -import org.apache.lucene.search.Scorer; -import org.apache.lucene.search.SimpleCollector; -import org.apache.lucene.search.Sort; -import org.apache.lucene.search.SortField; -import org.apache.lucene.search.TermQuery; -import org.apache.lucene.search.TermStatistics; -import org.apache.lucene.search.TimeLimitingCollector; -import org.apache.lucene.search.TopDocs; -import org.apache.lucene.search.TopDocsCollector; -import org.apache.lucene.search.TopFieldCollector; -import org.apache.lucene.search.TopFieldDocs; -import org.apache.lucene.search.TopScoreDocCollector; -import org.apache.lucene.search.TotalHitCountCollector; -import org.apache.lucene.search.Weight; import org.apache.lucene.store.Directory; import org.apache.lucene.util.Bits; import org.apache.lucene.util.BytesRef; @@ -128,10 +82,6 @@ import org.apache.solr.update.SolrIndexConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Function; -import com.google.common.base.Objects; -import com.google.common.collect.Iterables; - /** * SolrIndexSearcher adds schema awareness and caching functionality over {@link IndexSearcher}. * @@ -337,13 +287,12 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI documentCache = solrConfig.documentCacheConfig == null ? null : solrConfig.documentCacheConfig.newInstance(); if (documentCache != null) clist.add(documentCache); - if (solrConfig.userCacheConfigs == null) { + if (solrConfig.userCacheConfigs.isEmpty()) { cacheMap = NO_GENERIC_CACHES; } else { - cacheMap = new HashMap<>(solrConfig.userCacheConfigs.length); - for (CacheConfig userCacheConfig : solrConfig.userCacheConfigs) { - SolrCache cache = null; - if (userCacheConfig != null) cache = userCacheConfig.newInstance(); + cacheMap = new HashMap<>(solrConfig.userCacheConfigs.size()); + for (Map.Entry e : solrConfig.userCacheConfigs.entrySet()) { + SolrCache cache = e.getValue().newInstance(); if (cache != null) { cacheMap.put(cache.name(), cache); clist.add(cache); 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 41e32dd697f..c182495e91a 100644 --- a/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java +++ b/solr/core/src/test/org/apache/solr/core/TestSolrConfigHandler.java @@ -22,6 +22,7 @@ import java.io.StringReader; import java.lang.invoke.MethodHandles; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -35,8 +36,12 @@ import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.common.util.StrUtils; import org.apache.solr.common.util.Utils; +import org.apache.solr.handler.DumpRequestHandler; import org.apache.solr.handler.TestBlobHandler; import org.apache.solr.handler.TestSolrConfigHandlerConcurrent; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.search.SolrCache; import org.apache.solr.util.RestTestBase; import org.apache.solr.util.RestTestHarness; import org.eclipse.jetty.servlet.ServletHolder; @@ -449,7 +454,56 @@ public class TestSolrConfigHandler extends RestTestBase { assertEquals(2, initArgs.size()); assertTrue(((Map)initArgs.get(0)).containsKey("suggester")); assertTrue(((Map)initArgs.get(1)).containsKey("suggester")); - System.out.println(map); + + payload = "{\n" + + "'add-requesthandler' : { 'name' : '/dump101', 'class': " + + "'" + CacheTest.class.getName() + "' " + + ", 'startup' : 'lazy'}\n" + + "}"; + runConfigCommand(writeHarness, "/config?wt=json", payload); + + testForResponseElement(writeHarness, + testServerBaseUrl, + "/config/overlay?wt=json", + cloudSolrClient, + Arrays.asList("overlay", "requestHandler", "/dump101", "startup"), + "lazy", + 10); + + payload = "{\n" + + "'add-cache' : {name:'lfuCacheDecayFalse', class:'solr.search.LFUCache', size:10 ,initialSize:9 , timeDecay:false }," + + "'add-cache' : {name: 'perSegFilter', class: 'solr.search.LRUCache', size:10, initialSize:0 , autowarmCount:10}}"; + runConfigCommand(writeHarness, "/config?wt=json", payload); + + map = testForResponseElement(writeHarness, + testServerBaseUrl, + "/config/overlay?wt=json", + cloudSolrClient, + Arrays.asList("overlay", "cache", "lfuCacheDecayFalse", "class"), + "solr.search.LFUCache", + 10); + assertEquals("solr.search.LRUCache",getObjectByPath(map, true, ImmutableList.of("overlay", "cache", "perSegFilter", "class"))); + + map = getRespMap("/dump101?cacheNames=lfuCacheDecayFalse&cacheNames=perSegFilter&wt=json", writeHarness); + assertEquals("Actual output "+ Utils.toJSONString(map), "org.apache.solr.search.LRUCache",getObjectByPath(map, true, ImmutableList.of( "caches", "perSegFilter"))); + assertEquals("Actual output "+ Utils.toJSONString(map), "org.apache.solr.search.LFUCache",getObjectByPath(map, true, ImmutableList.of( "caches", "lfuCacheDecayFalse"))); + + } + + public static class CacheTest extends DumpRequestHandler { + @Override + public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException { + super.handleRequestBody(req, rsp); + String[] caches = req.getParams().getParams("cacheNames"); + if(caches != null && caches.length>0){ + HashMap m = new HashMap(); + rsp.add("caches", m); + for (String c : caches) { + SolrCache cache = req.getSearcher().getCache(c); + if(cache != null) m.put(c, cache.getClass().getName()); + } + } + } } public static Map testForResponseElement(RestTestHarness harness, From bc25a565d23a7f791272be02685e71217234704b Mon Sep 17 00:00:00 2001 From: Christine Poerschke Date: Wed, 10 Aug 2016 16:01:19 +0100 Subject: [PATCH 04/48] SOLR-9331: Remove ReRankQuery's length constructor argument and member. --- solr/CHANGES.txt | 2 ++ .../apache/solr/search/ReRankQParserPlugin.java | 14 ++++---------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 05252c33469..6755b706ad6 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -253,6 +253,8 @@ Other Changes * SOLR-9367: Improved TestInjection's randomization logic to use LuceneTestCase.random() (hossman) +* SOLR-9331: Remove ReRankQuery's length constructor argument and member. (Christine Poerschke) + ================== 6.1.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. diff --git a/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java b/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java index 3f0bb0e7ddb..a903968058a 100644 --- a/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java +++ b/solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java @@ -42,7 +42,6 @@ import org.apache.lucene.search.TopScoreDocCollector; import org.apache.lucene.search.Weight; import org.apache.lucene.util.BytesRef; import org.apache.solr.common.SolrException; -import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.handler.component.MergeStrategy; import org.apache.solr.handler.component.QueryElevationComponent; @@ -91,10 +90,7 @@ public class ReRankQParserPlugin extends QParserPlugin { double reRankWeight = localParams.getDouble(RERANK_WEIGHT, RERANK_WEIGHT_DEFAULT); - int start = params.getInt(CommonParams.START,CommonParams.START_DEFAULT); - int rows = params.getInt(CommonParams.ROWS,CommonParams.ROWS_DEFAULT); - int length = start+rows; - return new ReRankQuery(reRankQuery, reRankDocs, reRankWeight, length); + return new ReRankQuery(reRankQuery, reRankDocs, reRankWeight); } } @@ -121,7 +117,6 @@ public class ReRankQParserPlugin extends QParserPlugin { private Query mainQuery = defaultQuery; final private Query reRankQuery; final private int reRankDocs; - final private int length; final private double reRankWeight; final private Rescorer reRankQueryRescorer; private Map boostedPriority; @@ -142,11 +137,10 @@ public class ReRankQParserPlugin extends QParserPlugin { reRankDocs == rrq.reRankDocs; } - public ReRankQuery(Query reRankQuery, int reRankDocs, double reRankWeight, int length) { + public ReRankQuery(Query reRankQuery, int reRankDocs, double reRankWeight) { this.reRankQuery = reRankQuery; this.reRankDocs = reRankDocs; this.reRankWeight = reRankWeight; - this.length = length; this.reRankQueryRescorer = new ReRankQueryRescorer(reRankQuery, reRankWeight); } @@ -171,7 +165,7 @@ public class ReRankQParserPlugin extends QParserPlugin { } } - return new ReRankCollector(reRankDocs, length, reRankQueryRescorer, cmd, searcher, boostedPriority); + return new ReRankCollector(reRankDocs, len, reRankQueryRescorer, cmd, searcher, boostedPriority); } @Override @@ -188,7 +182,7 @@ public class ReRankQParserPlugin extends QParserPlugin { public Query rewrite(IndexReader reader) throws IOException { Query q = mainQuery.rewrite(reader); if (q != mainQuery) { - return new ReRankQuery(reRankQuery, reRankDocs, reRankWeight, length).wrap(q); + return new ReRankQuery(reRankQuery, reRankDocs, reRankWeight).wrap(q); } return super.rewrite(reader); } From ae60c74f8c6ea2f62e1870802accbcd73bbfdc48 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Thu, 11 Aug 2016 12:12:48 +0530 Subject: [PATCH 05/48] SOLR-9320: A REPLACENODE command to decommission an existing node with another new node and SOLR-9318 the DELETENODE command that deletes all replicas in a node --- solr/CHANGES.txt | 5 + .../org/apache/solr/cloud/DeleteNodeCmd.java | 86 +++++++++ .../OverseerCollectionConfigSetProcessor.java | 22 ++- .../OverseerCollectionMessageHandler.java | 160 +++++++++++++---- .../solr/cloud/OverseerTaskProcessor.java | 6 +- .../org/apache/solr/cloud/ReplaceNodeCmd.java | 164 ++++++++++++++++++ .../handler/admin/CollectionsHandler.java | 5 +- .../org/apache/solr/cloud/DeleteNodeTest.java | 75 ++++++++ .../apache/solr/cloud/ReplaceNodeTest.java | 104 +++++++++++ .../solrj/request/CollectionAdminRequest.java | 52 +++++- .../apache/solr/common/cloud/ZkNodeProps.java | 12 ++ .../solr/common/params/CollectionParams.java | 3 + 12 files changed, 650 insertions(+), 44 deletions(-) create mode 100644 solr/core/src/java/org/apache/solr/cloud/DeleteNodeCmd.java create mode 100644 solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java create mode 100644 solr/core/src/test/org/apache/solr/cloud/DeleteNodeTest.java create mode 100644 solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 4da2d0eef94..889611f6b9a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -100,6 +100,11 @@ New Features * SOLR-9275: XML QueryParser support (defType=xmlparser) now extensible via configuration. (Christine Poerschke) +* SOLR-9320: A REPLACENODE command to decommission an existing node with another new node + (noble, Nitin Sharma, Varun Thacker) + +* SOLR-9318: A DELETENODE command to delete all replicas in that node (noble, Nitin Sharma, Varun Thacker) + Bug Fixes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/cloud/DeleteNodeCmd.java b/solr/core/src/java/org/apache/solr/cloud/DeleteNodeCmd.java new file mode 100644 index 00000000000..cbcfa8847df --- /dev/null +++ b/solr/core/src/java/org/apache/solr/cloud/DeleteNodeCmd.java @@ -0,0 +1,86 @@ +/* + * 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.cloud; + + +import java.lang.invoke.MethodHandles; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.apache.solr.common.SolrException; +import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.ZkNodeProps; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.Utils; +import org.apache.zookeeper.KeeperException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class DeleteNodeCmd implements OverseerCollectionMessageHandler.Cmd { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private final OverseerCollectionMessageHandler ocmh; + + public DeleteNodeCmd(OverseerCollectionMessageHandler ocmh) { + this.ocmh = ocmh; + } + + @Override + public Object call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception { + ocmh.checkRequired(message, "node"); + String node = message.getStr("node"); + if (!state.liveNodesContain(node)) { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Source Node: " + node + " is not live"); + } + List sourceReplicas = ReplaceNodeCmd.getReplicasOfNode(node, state); + cleanupReplicas(results, state, sourceReplicas, ocmh); + return null; + } + + static void cleanupReplicas(NamedList results, + ClusterState clusterState, + List sourceReplicas, + OverseerCollectionMessageHandler ocmh) throws InterruptedException { + CountDownLatch cleanupLatch = new CountDownLatch(sourceReplicas.size()); + for (ZkNodeProps sourceReplica : sourceReplicas) { + log.info("deleting replica from from node {} ", Utils.toJSONString(sourceReplica)); + NamedList deleteResult = new NamedList(); + try { + ocmh.deleteReplica(clusterState, sourceReplica.plus("parallel", "true"), deleteResult, () -> { + cleanupLatch.countDown(); + if (deleteResult.get("failure") != null) { + synchronized (results) { + results.add("failure", "could not delete because " + deleteResult.get("failure") + " " + Utils.toJSONString(sourceReplica)); + } + } + }); + } catch (KeeperException e) { + log.info("Error deleting ", e); + cleanupLatch.countDown(); + } catch (Exception e) { + cleanupLatch.countDown(); + throw e; + } + } + log.info("Waiting for deletes to complete"); + cleanupLatch.await(5, TimeUnit.MINUTES); + } + + +} diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionConfigSetProcessor.java b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionConfigSetProcessor.java index f8f84462df0..8c7a056299e 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionConfigSetProcessor.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionConfigSetProcessor.java @@ -16,6 +16,10 @@ */ package org.apache.solr.cloud; +import java.io.IOException; + +import org.apache.commons.io.IOUtils; +import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.handler.component.ShardHandler; import org.apache.solr.handler.component.ShardHandlerFactory; @@ -83,12 +87,20 @@ public class OverseerCollectionConfigSetProcessor extends OverseerTaskProcessor zkStateReader, myId, shardHandlerFactory, adminPath, stats, overseer, overseerNodePrioritizer); final OverseerConfigSetMessageHandler configMessageHandler = new OverseerConfigSetMessageHandler( zkStateReader); - return message -> { - String operation = message.getStr(Overseer.QUEUE_OPERATION); - if (operation != null && operation.startsWith(CONFIGSETS_ACTION_PREFIX)) { - return configMessageHandler; + return new OverseerMessageHandlerSelector() { + @Override + public void close() throws IOException { + IOUtils.closeQuietly(collMessageHandler); + } + + @Override + public OverseerMessageHandler selectOverseerMessageHandler(ZkNodeProps message) { + String operation = message.getStr(Overseer.QUEUE_OPERATION); + if (operation != null && operation.startsWith(CONFIGSETS_ACTION_PREFIX)) { + return configMessageHandler; + } + return collMessageHandler; } - return collMessageHandler; }; } } diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java index 4e7e429bb75..908d35c3e6b 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java @@ -16,6 +16,7 @@ */ package org.apache.solr.cloud; +import java.io.Closeable; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.net.URI; @@ -35,8 +36,13 @@ import java.util.Optional; import java.util.Properties; import java.util.Random; import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.SynchronousQueue; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import com.google.common.collect.ImmutableMap; import org.apache.commons.lang.StringUtils; import org.apache.solr.client.solrj.SolrResponse; import org.apache.solr.client.solrj.SolrServerException; @@ -75,6 +81,7 @@ import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.ShardParams; +import org.apache.solr.common.util.ExecutorUtil; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.StrUtils; @@ -88,6 +95,7 @@ import org.apache.solr.handler.component.ShardHandlerFactory; import org.apache.solr.handler.component.ShardRequest; import org.apache.solr.handler.component.ShardResponse; import org.apache.solr.update.SolrIndexSplitter; +import org.apache.solr.util.DefaultSolrThreadFactory; import org.apache.solr.util.RTimer; import org.apache.solr.util.TimeOut; import org.apache.solr.util.stats.Snapshot; @@ -119,10 +127,12 @@ import static org.apache.solr.common.params.CollectionParams.CollectionAction.BA import static org.apache.solr.common.params.CollectionParams.CollectionAction.CREATE; import static org.apache.solr.common.params.CollectionParams.CollectionAction.CREATESHARD; import static org.apache.solr.common.params.CollectionParams.CollectionAction.DELETE; +import static org.apache.solr.common.params.CollectionParams.CollectionAction.DELETENODE; import static org.apache.solr.common.params.CollectionParams.CollectionAction.DELETEREPLICAPROP; import static org.apache.solr.common.params.CollectionParams.CollectionAction.DELETESHARD; import static org.apache.solr.common.params.CollectionParams.CollectionAction.MIGRATESTATEFORMAT; import static org.apache.solr.common.params.CollectionParams.CollectionAction.REMOVEROLE; +import static org.apache.solr.common.params.CollectionParams.CollectionAction.REPLACENODE; import static org.apache.solr.common.params.CommonAdminParams.ASYNC; import static org.apache.solr.common.params.CommonParams.NAME; import static org.apache.solr.common.util.StrUtils.formatString; @@ -132,7 +142,7 @@ import static org.apache.solr.common.util.Utils.makeMap; * A {@link OverseerMessageHandler} that handles Collections API related * overseer messages. */ -public class OverseerCollectionMessageHandler implements OverseerMessageHandler { +public class OverseerCollectionMessageHandler implements OverseerMessageHandler , Closeable { public static final String NUM_SLICES = "numShards"; @@ -172,7 +182,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler private Overseer overseer; private ShardHandlerFactory shardHandlerFactory; private String adminPath; - private ZkStateReader zkStateReader; + ZkStateReader zkStateReader; private String myId; private Overseer.Stats stats; private OverseerNodePrioritizer overseerPrioritizer; @@ -181,6 +191,9 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler // This is used for handling mutual exclusion of the tasks. final private LockTree lockTree = new LockTree(); + ExecutorService tpe = new ExecutorUtil.MDCAwareThreadPoolExecutor(5, 10, 0L, TimeUnit.MILLISECONDS, + new SynchronousQueue<>(), + new DefaultSolrThreadFactory("OverseerCollectionMessageHandlerThreadFactory")); static final Random RANDOM; static { @@ -193,6 +206,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler RANDOM = new Random(seed.hashCode()); } } + private final Map commandMap; public OverseerCollectionMessageHandler(ZkStateReader zkStateReader, String myId, final ShardHandlerFactory shardHandlerFactory, @@ -207,6 +221,11 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler this.stats = stats; this.overseer = overseer; this.overseerPrioritizer = overseerPrioritizer; + commandMap = new ImmutableMap.Builder() + .put(REPLACENODE, new ReplaceNodeCmd(this)) + .put(DELETENODE, new DeleteNodeCmd(this)) + .build() + ; } @Override @@ -244,7 +263,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler createShard(zkStateReader.getClusterState(), message, results); break; case DELETEREPLICA: - deleteReplica(zkStateReader.getClusterState(), message, results); + deleteReplica(zkStateReader.getClusterState(), message, results, null); break; case MIGRATE: migrate(zkStateReader.getClusterState(), message, results); @@ -256,7 +275,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler processRoleCommand(message, operation); break; case ADDREPLICA: - addReplica(zkStateReader.getClusterState(), message, results); + addReplica(zkStateReader.getClusterState(), message, results, null); break; case OVERSEERSTATUS: getOverseerStatus(message, results); @@ -294,9 +313,15 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler results.add("MOCK_FINISHED", System.currentTimeMillis()); break; } - default: - throw new SolrException(ErrorCode.BAD_REQUEST, "Unknown operation:" - + operation); + default: { + Cmd command = commandMap.get(action); + if (command != null) { + command.call(zkStateReader.getClusterState(),message, results); + } else { + throw new SolrException(ErrorCode.BAD_REQUEST, "Unknown operation:" + + operation); + } + } } } catch (Exception e) { String collName = message.getStr("collection"); @@ -590,12 +615,14 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler } @SuppressWarnings("unchecked") - private void deleteReplica(ClusterState clusterState, ZkNodeProps message, NamedList results) + void deleteReplica(ClusterState clusterState, ZkNodeProps message, NamedList results, Runnable onComplete) throws KeeperException, InterruptedException { + log.info("deleteReplica() : {}", Utils.toJSONString(message)); checkRequired(message, COLLECTION_PROP, SHARD_ID_PROP, REPLICA_PROP); String collectionName = message.getStr(COLLECTION_PROP); String shard = message.getStr(SHARD_ID_PROP); String replicaName = message.getStr(REPLICA_PROP); + boolean parallel = message.getBool("parallel", false); DocCollection coll = clusterState.getCollection(collectionName); Slice slice = coll.getSlice(shard); @@ -623,9 +650,9 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler ShardHandler shardHandler = shardHandlerFactory.getShardHandler(); String core = replica.getStr(ZkStateReader.CORE_NAME_PROP); String asyncId = message.getStr(ASYNC); - Map requestMap = null; + AtomicReference> requestMap = new AtomicReference<>(null); if (asyncId != null) { - requestMap = new HashMap<>(1, 1.0f); + requestMap.set(new HashMap<>(1, 1.0f)); } ModifiableSolrParams params = new ModifiableSolrParams(); @@ -636,19 +663,42 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler params.set(CoreAdminParams.DELETE_INSTANCE_DIR, message.getBool(CoreAdminParams.DELETE_INSTANCE_DIR, true)); params.set(CoreAdminParams.DELETE_DATA_DIR, message.getBool(CoreAdminParams.DELETE_DATA_DIR, true)); - sendShardRequest(replica.getNodeName(), params, shardHandler, asyncId, requestMap); + sendShardRequest(replica.getNodeName(), params, shardHandler, asyncId, requestMap.get()); + AtomicReference exp = new AtomicReference<>(); - processResponses(results, shardHandler, false, null, asyncId, requestMap); + Callable callable = () -> { + try { + processResponses(results, shardHandler, false, null, asyncId, requestMap.get()); - //check if the core unload removed the corenode zk entry - if (waitForCoreNodeGone(collectionName, shard, replicaName, 5000)) return; + //check if the core unload removed the corenode zk entry + if (waitForCoreNodeGone(collectionName, shard, replicaName, 5000)) return Boolean.TRUE; - // try and ensure core info is removed from cluster state - deleteCoreNode(collectionName, replicaName, replica, core); - if (waitForCoreNodeGone(collectionName, shard, replicaName, 30000)) return; - - throw new SolrException(ErrorCode.SERVER_ERROR, - "Could not remove replica : " + collectionName + "/" + shard + "/" + replicaName); + // try and ensure core info is removed from cluster state + deleteCoreNode(collectionName, replicaName, replica, core); + if (waitForCoreNodeGone(collectionName, shard, replicaName, 30000)) return Boolean.TRUE; + return Boolean.FALSE; + } catch (Exception e) { + results.add("failure", "Could not complete delete " + e.getMessage()); + throw e; + } finally { + if (onComplete != null) onComplete.run(); + } + }; + + if (!parallel) { + try { + if (!callable.call()) + throw new SolrException(ErrorCode.SERVER_ERROR, + "Could not remove replica : " + collectionName + "/" + shard + "/" + replicaName); + } catch (InterruptedException | KeeperException e) { + throw e; + } catch (Exception ex) { + throw new SolrException(ErrorCode.UNKNOWN, "Error waiting for corenode gone", ex); + } + + } else { + tpe.submit(callable); + } } private boolean waitForCoreNodeGone(String collectionName, String shard, String replicaName, int timeoutms) throws InterruptedException { @@ -679,7 +729,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler Overseer.getStateUpdateQueue(zkStateReader.getZkClient()).offer(Utils.toJSON(m)); } - private void checkRequired(ZkNodeProps message, String... props) { + void checkRequired(ZkNodeProps message, String... props) { for (String prop : props) { if(message.get(prop) == null){ throw new SolrException(ErrorCode.BAD_REQUEST, StrUtils.join(Arrays.asList(props),',') +" are required params" ); @@ -1137,7 +1187,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler if (asyncId != null) { propMap.put(ASYNC, asyncId); } - addReplica(clusterState, new ZkNodeProps(propMap), results); + addReplica(clusterState, new ZkNodeProps(propMap), results, null); } processResponses(results, shardHandler, true, "SPLITSHARD failed to create subshard leaders", asyncId, requestMap); @@ -1307,7 +1357,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler // now actually create replica cores on sub shard nodes for (Map replica : replicas) { - addReplica(clusterState, new ZkNodeProps(replica), results); + addReplica(clusterState, new ZkNodeProps(replica), results, null); } processResponses(results, shardHandler, true, "SPLITSHARD failed to create subshard replicas", asyncId, requestMap); @@ -1681,7 +1731,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler if(asyncId != null) { props.put(ASYNC, asyncId); } - addReplica(clusterState, new ZkNodeProps(props), results); + addReplica(clusterState, new ZkNodeProps(props), results, null); processResponses(results, shardHandler, true, "MIGRATE failed to create replica of " + "temporary collection in target leader node.", asyncId, requestMap); @@ -2110,12 +2160,14 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler } } - private void addReplica(ClusterState clusterState, ZkNodeProps message, NamedList results) + ZkNodeProps addReplica(ClusterState clusterState, ZkNodeProps message, NamedList results, Runnable onComplete) throws KeeperException, InterruptedException { + log.info("addReplica() : {}", Utils.toJSONString(message)); String collection = message.getStr(COLLECTION_PROP); String node = message.getStr(CoreAdminParams.NODE); String shard = message.getStr(SHARD_ID_PROP); String coreName = message.getStr(CoreAdminParams.NAME); + boolean parallel = message.getBool("parallel", false); if (StringUtils.isBlank(coreName)) { coreName = message.getStr(CoreAdminParams.PROPERTY_PREFIX + CoreAdminParams.NAME); } @@ -2138,7 +2190,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler node = getNodesForNewReplicas(clusterState, collection, shard, 1, node, overseer.getZkController().getCoreContainer()).get(0).nodeName; } - log.info("Node not provided, Identified {} for creating new replica", node); + log.info("Node Identified {} for creating new replica", node); if (!clusterState.liveNodesContain(node)) { throw new SolrException(ErrorCode.BAD_REQUEST, "Node: " + node + " is not live"); @@ -2161,10 +2213,14 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler if (!Overseer.isLegacy(zkStateReader)) { if (!skipCreateReplicaInClusterState) { - ZkNodeProps props = new ZkNodeProps(Overseer.QUEUE_OPERATION, ADDREPLICA.toLower(), ZkStateReader.COLLECTION_PROP, - collection, ZkStateReader.SHARD_ID_PROP, shard, ZkStateReader.CORE_NAME_PROP, coreName, - ZkStateReader.STATE_PROP, Replica.State.DOWN.toString(), ZkStateReader.BASE_URL_PROP, - zkStateReader.getBaseUrlForNodeName(node), ZkStateReader.NODE_NAME_PROP, node); + ZkNodeProps props = new ZkNodeProps( + Overseer.QUEUE_OPERATION, ADDREPLICA.toLower(), + ZkStateReader.COLLECTION_PROP, collection, + ZkStateReader.SHARD_ID_PROP, shard, + ZkStateReader.CORE_NAME_PROP, coreName, + ZkStateReader.STATE_PROP, Replica.State.DOWN.toString(), + ZkStateReader.BASE_URL_PROP, zkStateReader.getBaseUrlForNodeName(node), + ZkStateReader.NODE_NAME_PROP, node); Overseer.getStateUpdateQueue(zkStateReader.getZkClient()).offer(Utils.toJSON(props)); } params.set(CoreAdminParams.CORE_NODE_NAME, @@ -2204,9 +2260,28 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler Map requestMap = new HashMap<>(); sendShardRequest(node, params, shardHandler, asyncId, requestMap); - processResponses(results, shardHandler, true, "ADDREPLICA failed to create replica", asyncId, requestMap); + final String fnode = node; + final String fcoreName = coreName; - waitForCoreNodeName(collection, node, coreName); + Runnable runnable = () -> { + processResponses(results, shardHandler, true, "ADDREPLICA failed to create replica", asyncId, requestMap); + waitForCoreNodeName(collection, fnode, fcoreName); + if (onComplete != null) onComplete.run(); + }; + + if (!parallel) { + runnable.run(); + } else { + tpe.submit(runnable); + } + + + return new ZkNodeProps( + ZkStateReader.COLLECTION_PROP, collection, + ZkStateReader.SHARD_ID_PROP, shard, + ZkStateReader.CORE_NAME_PROP, coreName, + ZkStateReader.NODE_NAME_PROP, node + ); } private void processBackupAction(ZkNodeProps message, NamedList results) throws IOException, KeeperException, InterruptedException { @@ -2394,7 +2469,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler } addPropertyParams(message, propMap); - addReplica(clusterState, new ZkNodeProps(propMap), new NamedList()); + addReplica(clusterState, new ZkNodeProps(propMap), new NamedList(), null); } //refresh the location copy of collection state @@ -2443,7 +2518,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler } addPropertyParams(message, propMap); - addReplica(zkStateReader.getClusterState(), new ZkNodeProps(propMap), results); + addReplica(zkStateReader.getClusterState(), new ZkNodeProps(propMap), results, null); } } } @@ -2503,7 +2578,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler } return configName; } - + private void validateConfigOrThrowSolrException(String configName) throws KeeperException, InterruptedException { boolean isValid = zkStateReader.getZkClient().exists(ZkConfigManager.CONFIGS_ZKNODE + "/" + configName, true); if(!isValid) { @@ -2723,4 +2798,19 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler ); } + @Override + public void close() throws IOException { + if (tpe != null) { + if (!tpe.isShutdown()) { + ExecutorUtil.shutdownAndAwaitTermination(tpe); + } + } + } + + interface Cmd { + + Object call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception; + + } + } diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java index e3bc1f3af18..074cf163c4a 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskProcessor.java @@ -31,6 +31,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.Predicate; import com.google.common.collect.ImmutableSet; +import org.apache.commons.io.IOUtils; import org.apache.solr.client.solrj.SolrResponse; import org.apache.solr.cloud.OverseerTaskQueue.QueueEvent; import org.apache.solr.cloud.Overseer.LeaderStatus; @@ -115,7 +116,7 @@ public class OverseerTaskProcessor implements Runnable, Closeable { private final Object waitLock = new Object(); - private OverseerMessageHandlerSelector selector; + protected OverseerMessageHandlerSelector selector; private OverseerNodePrioritizer prioritizer; @@ -328,6 +329,7 @@ public class OverseerTaskProcessor implements Runnable, Closeable { ExecutorUtil.shutdownAndAwaitTermination(tpe); } } + IOUtils.closeQuietly(selector); } public static List getSortedOverseerNodeNames(SolrZkClient zk) throws KeeperException, InterruptedException { @@ -588,7 +590,7 @@ public class OverseerTaskProcessor implements Runnable, Closeable { * messages only) , or a different handler could be selected based on the * contents of the message. */ - public interface OverseerMessageHandlerSelector { + public interface OverseerMessageHandlerSelector extends Closeable { OverseerMessageHandler selectOverseerMessageHandler(ZkNodeProps message); } diff --git a/solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java b/solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java new file mode 100644 index 00000000000..0cfd0894ffe --- /dev/null +++ b/solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java @@ -0,0 +1,164 @@ +/* + * 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.cloud; + + +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.apache.solr.common.SolrException; +import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.cloud.Slice; +import org.apache.solr.common.cloud.ZkNodeProps; +import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.params.CoreAdminParams; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.Utils; +import org.apache.zookeeper.KeeperException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP; +import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP; + +public class ReplaceNodeCmd implements OverseerCollectionMessageHandler.Cmd { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + private final OverseerCollectionMessageHandler ocmh; + + public ReplaceNodeCmd(OverseerCollectionMessageHandler ocmh) { + this.ocmh = ocmh; + } + + @Override + public Object call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception { + ZkStateReader zkStateReader = ocmh.zkStateReader; + ocmh.checkRequired(message, "source", "target"); + String source = message.getStr("source"); + String target = message.getStr("target"); + boolean parallel = message.getBool("parallel", false); + ClusterState clusterState = zkStateReader.getClusterState(); + + if (!clusterState.liveNodesContain(source)) { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Source Node: " + source + " is not live"); + } + if (!clusterState.liveNodesContain(target)) { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Target Node: " + target + " is not live"); + } + List sourceReplicas = getReplicasOfNode(source, clusterState); + + List createdReplicas = new ArrayList<>(); + + AtomicBoolean anyOneFailed = new AtomicBoolean(false); + CountDownLatch countDownLatch = new CountDownLatch(sourceReplicas.size()); + + for (ZkNodeProps sourceReplica : sourceReplicas) { + NamedList nl = new NamedList(); + log.info("going to create replica {}", Utils.toJSONString(sourceReplica)); + ZkNodeProps msg = sourceReplica.plus("parallel", String.valueOf(parallel)).plus(CoreAdminParams.NODE, target); + final ZkNodeProps addedReplica = ocmh.addReplica(clusterState, + msg, nl, () -> { + countDownLatch.countDown(); + if (nl.get("failure") != null) { + log.warn("failed to create : " + Utils.toJSONString(msg)); + // one replica creation failed. Make the best attempt to + // delete all the replicas created so far in the target + // and exit + synchronized (results) { + results.add("failure", "Could not create copy of replica " + Utils.toJSONString(sourceReplica)); + anyOneFailed.set(true); + } + } else { + log.info("successfully created : " + Utils.toJSONString(msg)); + + } + }); + + if (addedReplica != null) { + createdReplicas.add(addedReplica); + } + } + + log.info("Waiting for creates to complete "); + countDownLatch.await(5, TimeUnit.MINUTES); + log.info("Waiting over for creates to complete "); + + if (anyOneFailed.get()) { + log.info("failed to create some cores delete all " + Utils.toJSONString(createdReplicas)); + CountDownLatch cleanupLatch = new CountDownLatch(createdReplicas.size()); + for (ZkNodeProps createdReplica : createdReplicas) { + NamedList deleteResult = new NamedList(); + try { + ocmh.deleteReplica(zkStateReader.getClusterState(), createdReplica.plus("parallel", "true"), deleteResult, () -> { + cleanupLatch.countDown(); + if (deleteResult.get("failure") != null) { + synchronized (results) { + results.add("failure", "could not cleanup, because : " + deleteResult.get("failure") + " " + Utils.toJSONString(createdReplica)); + } + } + }); + } catch (KeeperException e) { + cleanupLatch.countDown(); + log.info("Error deleting ", e); + } catch (Exception e) { + log.error("Unknown Error deleteing", e); + cleanupLatch.countDown(); + throw e; + } + } + cleanupLatch.await(5, TimeUnit.MINUTES); + return null; + } + + + // we have reached this far means all replicas could be recreated + //now cleanup the replicas in the source node + DeleteNodeCmd.cleanupReplicas(results, state, sourceReplicas, ocmh); + results.add("success", "REPLACENODE completed successfully from : " + source + " to : " + target); + return null; + } + + static List getReplicasOfNode(String source, ClusterState state) { + List sourceReplicas = new ArrayList<>(); + for (Map.Entry e : state.getCollectionsMap().entrySet()) { + for (Slice slice : e.getValue().getSlices()) { + for (Replica replica : slice.getReplicas()) { + if (source.equals(replica.getNodeName())) { + ZkNodeProps props = new ZkNodeProps( + COLLECTION_PROP, e.getKey(), + SHARD_ID_PROP, slice.getName(), + ZkStateReader.CORE_NAME_PROP, replica.getCoreName(), + ZkStateReader.REPLICA_PROP, replica.getName(), + CoreAdminParams.NODE, source); + log.info("src_core : {}", Utils.toJSONString(props)); + sourceReplicas.add(props + ); + } + } + } + } + return sourceReplicas; + } +} diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java index cb7279080ea..2e906aed55b 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java @@ -781,7 +781,10 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission req.getParams().getAll(params, COLL_CONF, REPLICATION_FACTOR, MAX_SHARDS_PER_NODE, STATE_FORMAT, AUTO_ADD_REPLICAS); copyPropertiesWithPrefix(req.getParams(), params, COLL_PROP_PREFIX); return params; - }); + }), + + REPLACENODE_OP(REPLACENODE, (req, rsp, h) -> req.getParams().required().getAll(req.getParams().getAll(null, "parallel"), "source", "target")), + DELETENODE_OP(DELETENODE, (req, rsp, h) -> req.getParams().required().getAll(null, "node")); public final CollectionOp fun; CollectionAction action; long timeOut; diff --git a/solr/core/src/test/org/apache/solr/cloud/DeleteNodeTest.java b/solr/core/src/test/org/apache/solr/cloud/DeleteNodeTest.java new file mode 100644 index 00000000000..8d2f6f25a78 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cloud/DeleteNodeTest.java @@ -0,0 +1,75 @@ +/* + * 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.cloud; + + +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Set; + +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.response.RequestStatusState; +import org.apache.solr.common.util.StrUtils; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class DeleteNodeTest extends SolrCloudTestCase { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + @BeforeClass + public static void setupCluster() throws Exception { + configureCluster(6) + .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-dynamic").resolve("conf")) + .configure(); + } + + protected String getSolrXml() { + return "solr.xml"; + } + + @Test + public void test() throws Exception { + cluster.waitForAllNodes(5000); + CloudSolrClient cloudClient = cluster.getSolrClient(); + String coll = "deletenodetest_coll"; + Set liveNodes = cloudClient.getZkStateReader().getClusterState().getLiveNodes(); + ArrayList l = new ArrayList<>(liveNodes); + Collections.shuffle(l, random()); + CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(coll, "conf1", 5, 2); + create.setCreateNodeSet(StrUtils.join(l, ',')).setMaxShardsPerNode(3); + cloudClient.request(create); + String node2bdecommissioned = l.get(0); + new CollectionAdminRequest.DeleteNode(node2bdecommissioned).processAsync("003", cloudClient); + CollectionAdminRequest.RequestStatus requestStatus = CollectionAdminRequest.requestStatus("003"); + boolean success = false; + for (int i = 0; i < 200; i++) { + CollectionAdminRequest.RequestStatusResponse rsp = requestStatus.process(cloudClient); + if (rsp.getRequestStatus() == RequestStatusState.COMPLETED) { + success = true; + break; + } + assertFalse(rsp.getRequestStatus() == RequestStatusState.FAILED); + Thread.sleep(50); + } + assertTrue(success); + } +} diff --git a/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java b/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java new file mode 100644 index 00000000000..1c7575d5225 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java @@ -0,0 +1,104 @@ +/* + * 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.cloud; + + +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Set; + +import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.CoreAdminRequest; +import org.apache.solr.client.solrj.response.CoreAdminResponse; +import org.apache.solr.client.solrj.response.RequestStatusState; +import org.apache.solr.common.util.StrUtils; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ReplaceNodeTest extends SolrCloudTestCase { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + @BeforeClass + public static void setupCluster() throws Exception { + configureCluster(6) + .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-dynamic").resolve("conf")) + .configure(); + } + + protected String getSolrXml() { + return "solr.xml"; + } + + @Test + public void test() throws Exception { + cluster.waitForAllNodes(5000); + String coll = "replacenodetest_coll"; + log.info("total_jettys: " + cluster.getJettySolrRunners().size()); + + CloudSolrClient cloudClient = cluster.getSolrClient(); + Set liveNodes = cloudClient.getZkStateReader().getClusterState().getLiveNodes(); + ArrayList l = new ArrayList<>(liveNodes); + Collections.shuffle(l, random()); + String emptyNode = l.remove(0); + String node2bdecommissioned = l.get(0); + CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(coll, "conf1", 5, 2); + create.setCreateNodeSet(StrUtils.join(l, ',')).setMaxShardsPerNode(3); + cloudClient.request(create); + log.info("excluded_node : {} ", emptyNode); + new CollectionAdminRequest.ReplaceNode(node2bdecommissioned, emptyNode).processAsync("000", cloudClient); + CollectionAdminRequest.RequestStatus requestStatus = CollectionAdminRequest.requestStatus("000"); + boolean success = false; + for (int i = 0; i < 200; i++) { + CollectionAdminRequest.RequestStatusResponse rsp = requestStatus.process(cloudClient); + if (rsp.getRequestStatus() == RequestStatusState.COMPLETED) { + success = true; + break; + } + assertFalse(rsp.getRequestStatus() == RequestStatusState.FAILED); + Thread.sleep(50); + } + assertTrue(success); + try (HttpSolrClient coreclient = getHttpSolrClient(cloudClient.getZkStateReader().getBaseUrlForNodeName(node2bdecommissioned))) { + CoreAdminResponse status = CoreAdminRequest.getStatus(null, coreclient); + assertTrue(status.getCoreStatus().size() == 0); + } + + //let's do it back + new CollectionAdminRequest.ReplaceNode(emptyNode, node2bdecommissioned).setParallel(Boolean.TRUE).processAsync("001", cloudClient); + requestStatus = CollectionAdminRequest.requestStatus("001"); + + for (int i = 0; i < 200; i++) { + CollectionAdminRequest.RequestStatusResponse rsp = requestStatus.process(cloudClient); + if (rsp.getRequestStatus() == RequestStatusState.COMPLETED) { + success = true; + break; + } + assertFalse(rsp.getRequestStatus() == RequestStatusState.FAILED); + Thread.sleep(50); + } + assertTrue(success); + try (HttpSolrClient coreclient = getHttpSolrClient(cloudClient.getZkStateReader().getBaseUrlForNodeName(emptyNode))) { + CoreAdminResponse status = CoreAdminRequest.getStatus(null, coreclient); + assertTrue(status.getCoreStatus().size() == 0); + } + } +} diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java index 7bc9e4fd931..0a0a191ab1d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java @@ -112,7 +112,7 @@ public abstract class CollectionAdminRequest * @deprecated Use {@link #processAsync(String, SolrClient)} or {@link #processAsync(SolrClient)} */ @Deprecated - public abstract AsyncCollectionAdminRequest setAsyncId(String id); + public AsyncCollectionAdminRequest setAsyncId(String id){return this;}; /** * Process this request asynchronously, generating and returning a request id @@ -491,6 +491,56 @@ public abstract class CollectionAdminRequest } } + public static class DeleteNode extends AsyncCollectionAdminRequest { + String node; + + /** + * @param node The node to be deleted + */ + public DeleteNode(String node) { + super(CollectionAction.DELETENODE); + this.node = node; + } + @Override + public SolrParams getParams() { + ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); + params.set("node", node); + return params; + } + + + } + + public static class ReplaceNode extends AsyncCollectionAdminRequest { + String source, target; + Boolean parallel; + + /** + * @param source node to be cleaned up + * @param target node where the new replicas are to be created + */ + public ReplaceNode(String source, String target) { + super(CollectionAction.REPLACENODE); + this.source = source; + this.target = target; + } + + public ReplaceNode setParallel(Boolean flag) { + this.parallel = flag; + return this; + } + + @Override + public SolrParams getParams() { + ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); + params.set("source", source); + params.set("target", target); + if (parallel != null) params.set("parallel", parallel.toString()); + return params; + } + + } + /* * Returns a RebalanceLeaders object to rebalance leaders for a collection */ diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java index 320ad132583..94a673e9941 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java @@ -20,6 +20,7 @@ import org.apache.solr.common.util.Utils; import org.noggit.JSONUtil; import org.noggit.JSONWriter; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -40,6 +41,17 @@ public class ZkNodeProps implements JSONWriter.Writable { // Always wrapping introduces a memory leak. } + public ZkNodeProps plus(String key , Object val) { + return plus(Collections.singletonMap(key,val)); + } + + public ZkNodeProps plus(Map newVals) { + LinkedHashMap copy = new LinkedHashMap<>(propMap); + if (newVals == null || newVals.isEmpty()) return new ZkNodeProps(copy); + copy.putAll(newVals); + return new ZkNodeProps(copy); + } + /** * Constructor that populates the from array of Strings in form key1, value1, diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CollectionParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CollectionParams.java index e38ab4fbb11..f10f0895b2e 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/CollectionParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/CollectionParams.java @@ -96,6 +96,9 @@ public interface CollectionParams { // but the overseer is aware of these tasks MOCK_COLL_TASK(false, LockLevel.COLLECTION), MOCK_SHARD_TASK(false, LockLevel.SHARD), + //TODO when we have a node level lock use it here + REPLACENODE(true, LockLevel.NONE), + DELETENODE(true, LockLevel.NONE), MOCK_REPLICA_TASK(false, LockLevel.REPLICA) ; public final boolean isWrite; From f82c3b1206011776c55867fb2b5027b824f99812 Mon Sep 17 00:00:00 2001 From: Shalin Shekhar Mangar Date: Fri, 12 Aug 2016 17:33:32 +0530 Subject: [PATCH 06/48] SOLR-9405: ConcurrentModificationException in ZkStateReader.getStateWatchers --- solr/CHANGES.txt | 3 ++ .../solr/common/cloud/ZkStateReader.java | 14 +++-- .../cloud/TestCollectionStateWatchers.java | 51 ++++++++++++++----- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 518f63a10d9..b2a384b2ce4 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -203,6 +203,9 @@ Bug Fixes * SOLR-9397: Config API does not support adding caches (noble) +* SOLR-9405: ConcurrentModificationException in ZkStateReader.getStateWatchers. + (Alan Woodward, Edward Ribeiro, shalin) + Optimizations ---------------------- diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java index 9df4a76aa8e..a3de324e0a7 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java @@ -147,7 +147,7 @@ public class ZkStateReader implements Closeable { private class CollectionWatch { int coreRefCount = 0; - Set stateWatchers = new HashSet<>(); + Set stateWatchers = ConcurrentHashMap.newKeySet(); public boolean canBeRemoved() { return coreRefCount + stateWatchers.size() == 0; @@ -1273,10 +1273,14 @@ public class ZkStateReader implements Closeable { /* package-private for testing */ Set getStateWatchers(String collection) { - CollectionWatch watch = collectionWatches.get(collection); - if (watch == null) - return null; - return new HashSet<>(watch.stateWatchers); + final Set watchers = new HashSet<>(); + collectionWatches.compute(collection, (k, v) -> { + if (v != null) { + watchers.addAll(v.stateWatchers); + } + return v; + }); + return watchers; } // returns true if the state has changed diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java b/solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java index d959aa83c9b..fca0e35728c 100644 --- a/solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java +++ b/solr/solrj/src/test/org/apache/solr/common/cloud/TestCollectionStateWatchers.java @@ -19,8 +19,9 @@ package org.apache.solr.common.cloud; import java.lang.invoke.MethodHandles; import java.util.HashMap; -import java.util.Set; +import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -81,6 +82,31 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { }); } + private static void waitFor(String message, long timeout, TimeUnit unit, Callable predicate) + throws InterruptedException, ExecutionException { + Future future = executor.submit(() -> { + try { + while (true) { + if (predicate.call()) + return true; + TimeUnit.MILLISECONDS.sleep(10); + } + } + catch (InterruptedException e) { + return false; + } + }); + try { + if (future.get(timeout, unit) == true) { + return; + } + } + catch (TimeoutException e) { + // pass failure message on + } + future.cancel(true); + fail(message); + } @Test public void testSimpleCollectionWatch() throws Exception { @@ -113,9 +139,8 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { cluster.stopJettySolrRunner(random().nextInt(cluster.getJettySolrRunners().size())); assertTrue("CollectionStateWatcher was never notified of cluster change", latch.await(MAX_WAIT_TIMEOUT, TimeUnit.SECONDS)); - Set watchers = client.getZkStateReader().getStateWatchers("testcollection"); - assertTrue("CollectionStateWatcher wasn't cleared after completion", - watchers == null || watchers.size() == 0); + waitFor("CollectionStateWatcher wasn't cleared after completion", 1, TimeUnit.SECONDS, + () -> client.getZkStateReader().getStateWatchers("testcollection").isEmpty()); } @@ -144,8 +169,8 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { assertTrue("CollectionStateWatcher isn't called when registering for already-watched collection", latch.await(MAX_WAIT_TIMEOUT, TimeUnit.SECONDS)); - assertEquals("CollectionStateWatcher should be removed", - 1, client.getZkStateReader().getStateWatchers("currentstate").size()); + waitFor("CollectionStateWatcher should be removed", 1, TimeUnit.SECONDS, + () -> client.getZkStateReader().getStateWatchers("currentstate").size() == 1); } @Test @@ -189,9 +214,8 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { expectThrows(TimeoutException.class, () -> { client.waitForState("nosuchcollection", 1, TimeUnit.SECONDS, ((liveNodes, collectionState) -> false)); }); - Set watchers = client.getZkStateReader().getStateWatchers("nosuchcollection"); - assertTrue("Watchers for collection should be removed after timeout", - watchers == null || watchers.size() == 0); + waitFor("Watchers for collection should be removed after timeout", 1, TimeUnit.SECONDS, + () -> client.getZkStateReader().getStateWatchers("nosuchcollection").isEmpty()); } @@ -229,18 +253,17 @@ public class TestCollectionStateWatchers extends SolrCloudTestCase { } @Test - public void testWatcherIsRemovedAfterTimeout() { + public void testWatcherIsRemovedAfterTimeout() throws Exception { CloudSolrClient client = cluster.getSolrClient(); assertTrue("There should be no watchers for a non-existent collection!", - client.getZkStateReader().getStateWatchers("no-such-collection") == null); + client.getZkStateReader().getStateWatchers("no-such-collection").isEmpty()); expectThrows(TimeoutException.class, () -> { client.waitForState("no-such-collection", 10, TimeUnit.MILLISECONDS, (n, c) -> DocCollection.isFullyActive(n, c, 1, 1)); }); - Set watchers = client.getZkStateReader().getStateWatchers("no-such-collection"); - assertTrue("Watchers for collection should be removed after timeout", - watchers == null || watchers.size() == 0); + waitFor("Watchers for collection should be removed after timeout", 1, TimeUnit.SECONDS, + () -> client.getZkStateReader().getStateWatchers("no-such-collection").isEmpty()); } From 633a89c0376e3db5b8c7efe325cdc3a409e250e5 Mon Sep 17 00:00:00 2001 From: Christine Poerschke Date: Fri, 12 Aug 2016 13:24:52 +0100 Subject: [PATCH 07/48] Fix system requirements typo. --- solr/site/SYSTEM_REQUIREMENTS.mdtext | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/site/SYSTEM_REQUIREMENTS.mdtext b/solr/site/SYSTEM_REQUIREMENTS.mdtext index 2c0fa87981e..bd41bb217b0 100644 --- a/solr/site/SYSTEM_REQUIREMENTS.mdtext +++ b/solr/site/SYSTEM_REQUIREMENTS.mdtext @@ -1,6 +1,6 @@ # System Requirements -Apache Solr runs of Java 8 or greater. +Apache Solr runs on Java 8 or greater. It is also recommended to always use the latest update version of your Java VM, because bugs may affect Solr. An overview of known JVM bugs From f20e2f3a941c8ffd3f4bac5607bb4b5f782cc29d Mon Sep 17 00:00:00 2001 From: Alexandre Rafalovitch Date: Fri, 12 Aug 2016 23:05:22 +1000 Subject: [PATCH 08/48] SOLR-9232: Fix Swap Cores in Admin UI --- solr/CHANGES.txt | 2 ++ solr/webapp/web/js/angular/controllers/cores.js | 8 ++++---- solr/webapp/web/js/angular/services.js | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index b2a384b2ce4..ee5b4a70f1a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -206,6 +206,8 @@ Bug Fixes * SOLR-9405: ConcurrentModificationException in ZkStateReader.getStateWatchers. (Alan Woodward, Edward Ribeiro, shalin) +* SOLR-9232: Admin UI now fully implements Swap Cores interface (Alexandre Rafalovitch) + Optimizations ---------------------- diff --git a/solr/webapp/web/js/angular/controllers/cores.js b/solr/webapp/web/js/angular/controllers/cores.js index 347dbf4c1f8..41863f2b591 100644 --- a/solr/webapp/web/js/angular/controllers/cores.js +++ b/solr/webapp/web/js/angular/controllers/cores.js @@ -129,15 +129,15 @@ solrAdminApp.controller('CoreAdminController', }; $scope.swapCores = function() { - if ($scope.swapOther) { - $swapMessage = "Please select a core to swap with"; + if (!$scope.swapOther) { + $scope.swapMessage = "Please select a core to swap with"; } else if ($scope.swapOther == $scope.selectedCore) { - $swapMessage = "Cannot swap with the same core"; + $scope.swapMessage = "Cannot swap with the same core"; } else { Cores.swap({core: $scope.selectedCore, other: $scope.swapOther}, function(data) { $location.path("/~cores/" + $scope.swapOther); delete $scope.swapOther; - $scope.cancelSwap(); + $scope.cancelSwapCores(); }); } }; diff --git a/solr/webapp/web/js/angular/services.js b/solr/webapp/web/js/angular/services.js index 014d36ba525..f050c9b7253 100644 --- a/solr/webapp/web/js/angular/services.js +++ b/solr/webapp/web/js/angular/services.js @@ -47,7 +47,7 @@ solrAdminServices.factory('System', "add": {params:{action: "CREATE"}}, "unload": {params:{action: "UNLOAD", core: "@core"}}, "rename": {params:{action: "RENAME"}}, - "swap": {params:{}}, + "swap": {params:{action: "SWAP"}}, "reload": {method: "GET", params:{action:"RELOAD", core: "@core"}, headers:{doNotIntercept: "true"}}, "optimize": {params:{}} }); From cea8a488f0d48041abb4be0dbe29ab81f04522bf Mon Sep 17 00:00:00 2001 From: Alexandre Rafalovitch Date: Sat, 13 Aug 2016 00:10:35 +1000 Subject: [PATCH 09/48] SOLR-8715: Admin UI - Fix schema special case --- solr/CHANGES.txt | 2 ++ solr/webapp/web/js/angular/controllers/schema.js | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index ee5b4a70f1a..5d4e2a64bf7 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -208,6 +208,8 @@ Bug Fixes * SOLR-9232: Admin UI now fully implements Swap Cores interface (Alexandre Rafalovitch) +* SOLR-8715: Admin UI's Schema screen now works for fields with stored=false and some content indexed (Alexandre Rafalovitch) + Optimizations ---------------------- diff --git a/solr/webapp/web/js/angular/controllers/schema.js b/solr/webapp/web/js/angular/controllers/schema.js index 22f752e7b75..ee23bd7931e 100644 --- a/solr/webapp/web/js/angular/controllers/schema.js +++ b/solr/webapp/web/js/angular/controllers/schema.js @@ -477,6 +477,10 @@ var getFieldProperties = function(data, core, is, field) { var row = display.rows[i]; row.cells = []; + if (!row.flags) { + continue; // Match the special case in the LukeRequestHandler + } + for (var j in display.columns) { var flag = display.columns[j].key; row.cells.push({key: flag, value: row.flags.indexOf(flag)>=0}); From 48cc5999369a1f99af159aa5eb756f5c6f118594 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Fri, 12 Aug 2016 19:54:33 +0200 Subject: [PATCH 10/48] LUCENE-7409: Changed MMapDirectory's unmapping to work safer, but still with no guarantees --- lucene/CHANGES.txt | 7 + .../apache/lucene/store/ByteBufferGuard.java | 130 ++++++++++++++++++ .../lucene/store/ByteBufferIndexInput.java | 104 ++++---------- .../apache/lucene/store/MMapDirectory.java | 8 +- .../lucene/store/TestMmapDirectory.java | 36 +++++ 5 files changed, 207 insertions(+), 78 deletions(-) create mode 100644 lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index c5db1438d8b..c43128cf245 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -142,6 +142,13 @@ Improvements because the ICU word-breaking algorithm has some issues. This allows for the previous tokenization used before Lucene 5. (AM, Robert Muir) +* LUCENE-7409: Changed MMapDirectory's unmapping to work safer, but still with + no guarantees. This uses a store-store barrier and yields the current thread + before unmapping to allow in-flight requests to finish. The new code no longer + uses WeakIdentityMap as it delegates all ByteBuffer reads throgh a new + ByteBufferGuard wrapper that is shared between all ByteBufferIndexInput clones. + (Robert Muir, Uwe Schindler) + Optimizations * LUCENE-7330, LUCENE-7339: Speed up conjunction queries. (Adrien Grand) diff --git a/lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java b/lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java new file mode 100644 index 00000000000..2e7ce2650c1 --- /dev/null +++ b/lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java @@ -0,0 +1,130 @@ +/* + * 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.lucene.store; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * A guard that is created for every {@link ByteBufferIndexInput} that tries on best effort + * to reject any access to the {@link ByteBuffer} behind, once it is unmapped. A single instance + * of this is used for the original and all clones, so once the original is closed and unmapped + * all clones also throw {@link AlreadyClosedException}, triggered by a {@link NullPointerException}. + *

+ * This code uses the trick that is also used in + * {@link java.lang.invoke.MutableCallSite#syncAll(java.lang.invoke.MutableCallSite[])} to + * invalidate switch points. It also yields the current thread to give other threads a chance + * to finish in-flight requests... + */ +final class ByteBufferGuard { + + /** + * Pass in an implementation of this interface to cleanup ByteBuffers. + * MMapDirectory implements this to allow unmapping of bytebuffers with private Java APIs. + */ + @FunctionalInterface + static interface BufferCleaner { + void freeBuffer(String resourceDescription, ByteBuffer b) throws IOException; + } + + private final String resourceDescription; + private final BufferCleaner cleaner; + + /** not volatile, we use store-store barrier! */ + private boolean invalidated = false; + + /** the actual store-store barrier. */ + private final AtomicInteger barrier = new AtomicInteger(); + + /** + * Creates an instance to be used for a single {@link ByteBufferIndexInput} which + * must be shared by all of its clones. + */ + public ByteBufferGuard(String resourceDescription, BufferCleaner cleaner) { + this.resourceDescription = resourceDescription; + this.cleaner = cleaner; + } + + /** + * Invalidates this guard and unmaps (if supported). + */ + public void invalidateAndUnmap(ByteBuffer... bufs) throws IOException { + if (cleaner != null) { + invalidated = true; + // this should trigger a happens-before - so flushes all caches + barrier.lazySet(0); + Thread.yield(); + for (ByteBuffer b : bufs) { + cleaner.freeBuffer(resourceDescription, b); + } + } + } + + private void ensureValid() { + if (invalidated) { + // this triggers an AlreadyClosedException in ByteBufferIndexInput: + throw new NullPointerException(); + } + } + + public void getBytes(ByteBuffer receiver, byte[] dst, int offset, int length) { + ensureValid(); + receiver.get(dst, offset, length); + } + + public byte getByte(ByteBuffer receiver) { + ensureValid(); + return receiver.get(); + } + + public short getShort(ByteBuffer receiver) { + ensureValid(); + return receiver.getShort(); + } + + public int getInt(ByteBuffer receiver) { + ensureValid(); + return receiver.getInt(); + } + + public long getLong(ByteBuffer receiver) { + ensureValid(); + return receiver.getLong(); + } + + public byte getByte(ByteBuffer receiver, int pos) { + ensureValid(); + return receiver.get(pos); + } + + public short getShort(ByteBuffer receiver, int pos) { + ensureValid(); + return receiver.getShort(pos); + } + + public int getInt(ByteBuffer receiver, int pos) { + ensureValid(); + return receiver.getInt(pos); + } + + public long getLong(ByteBuffer receiver, int pos) { + ensureValid(); + return receiver.getLong(pos); + } + +} diff --git a/lucene/core/src/java/org/apache/lucene/store/ByteBufferIndexInput.java b/lucene/core/src/java/org/apache/lucene/store/ByteBufferIndexInput.java index 8e8ef90655a..0f6c733410b 100644 --- a/lucene/core/src/java/org/apache/lucene/store/ByteBufferIndexInput.java +++ b/lucene/core/src/java/org/apache/lucene/store/ByteBufferIndexInput.java @@ -21,9 +21,6 @@ import java.io.EOFException; import java.io.IOException; import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; -import java.util.Iterator; - -import org.apache.lucene.util.WeakIdentityMap; /** * Base IndexInput implementation that uses an array @@ -37,35 +34,32 @@ import org.apache.lucene.util.WeakIdentityMap; * are a power-of-two (chunkSizePower). */ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessInput { - protected final BufferCleaner cleaner; protected final long length; protected final long chunkSizeMask; protected final int chunkSizePower; + protected final ByteBufferGuard guard; protected ByteBuffer[] buffers; protected int curBufIndex = -1; protected ByteBuffer curBuf; // redundant for speed: buffers[curBufIndex] protected boolean isClone = false; - protected final WeakIdentityMap clones; - public static ByteBufferIndexInput newInstance(String resourceDescription, ByteBuffer[] buffers, long length, int chunkSizePower, BufferCleaner cleaner, boolean trackClones) { - final WeakIdentityMap clones = trackClones ? WeakIdentityMap.newConcurrentHashMap() : null; + public static ByteBufferIndexInput newInstance(String resourceDescription, ByteBuffer[] buffers, long length, int chunkSizePower, ByteBufferGuard guard) { if (buffers.length == 1) { - return new SingleBufferImpl(resourceDescription, buffers[0], length, chunkSizePower, cleaner, clones); + return new SingleBufferImpl(resourceDescription, buffers[0], length, chunkSizePower, guard); } else { - return new MultiBufferImpl(resourceDescription, buffers, 0, length, chunkSizePower, cleaner, clones); + return new MultiBufferImpl(resourceDescription, buffers, 0, length, chunkSizePower, guard); } } - ByteBufferIndexInput(String resourceDescription, ByteBuffer[] buffers, long length, int chunkSizePower, BufferCleaner cleaner, WeakIdentityMap clones) { + ByteBufferIndexInput(String resourceDescription, ByteBuffer[] buffers, long length, int chunkSizePower, ByteBufferGuard guard) { super(resourceDescription); this.buffers = buffers; this.length = length; this.chunkSizePower = chunkSizePower; this.chunkSizeMask = (1L << chunkSizePower) - 1L; - this.clones = clones; - this.cleaner = cleaner; + this.guard = guard; assert chunkSizePower >= 0 && chunkSizePower <= 30; assert (length >>> chunkSizePower) < Integer.MAX_VALUE; } @@ -73,7 +67,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public final byte readByte() throws IOException { try { - return curBuf.get(); + return guard.getByte(curBuf); } catch (BufferUnderflowException e) { do { curBufIndex++; @@ -83,7 +77,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn curBuf = buffers[curBufIndex]; curBuf.position(0); } while (!curBuf.hasRemaining()); - return curBuf.get(); + return guard.getByte(curBuf); } catch (NullPointerException npe) { throw new AlreadyClosedException("Already closed: " + this); } @@ -92,11 +86,11 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public final void readBytes(byte[] b, int offset, int len) throws IOException { try { - curBuf.get(b, offset, len); + guard.getBytes(curBuf, b, offset, len); } catch (BufferUnderflowException e) { int curAvail = curBuf.remaining(); while (len > curAvail) { - curBuf.get(b, offset, curAvail); + guard.getBytes(curBuf, b, offset, curAvail); len -= curAvail; offset += curAvail; curBufIndex++; @@ -107,7 +101,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn curBuf.position(0); curAvail = curBuf.remaining(); } - curBuf.get(b, offset, len); + guard.getBytes(curBuf, b, offset, len); } catch (NullPointerException npe) { throw new AlreadyClosedException("Already closed: " + this); } @@ -116,7 +110,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public final short readShort() throws IOException { try { - return curBuf.getShort(); + return guard.getShort(curBuf); } catch (BufferUnderflowException e) { return super.readShort(); } catch (NullPointerException npe) { @@ -127,7 +121,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public final int readInt() throws IOException { try { - return curBuf.getInt(); + return guard.getInt(curBuf); } catch (BufferUnderflowException e) { return super.readInt(); } catch (NullPointerException npe) { @@ -138,7 +132,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public final long readLong() throws IOException { try { - return curBuf.getLong(); + return guard.getLong(curBuf); } catch (BufferUnderflowException e) { return super.readLong(); } catch (NullPointerException npe) { @@ -181,7 +175,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn public byte readByte(long pos) throws IOException { try { final int bi = (int) (pos >> chunkSizePower); - return buffers[bi].get((int) (pos & chunkSizeMask)); + return guard.getByte(buffers[bi], (int) (pos & chunkSizeMask)); } catch (IndexOutOfBoundsException ioobe) { throw new EOFException("seek past EOF: " + this); } catch (NullPointerException npe) { @@ -207,7 +201,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn public short readShort(long pos) throws IOException { final int bi = (int) (pos >> chunkSizePower); try { - return buffers[bi].getShort((int) (pos & chunkSizeMask)); + return guard.getShort(buffers[bi], (int) (pos & chunkSizeMask)); } catch (IndexOutOfBoundsException ioobe) { // either it's a boundary, or read past EOF, fall back: setPos(pos, bi); @@ -221,7 +215,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn public int readInt(long pos) throws IOException { final int bi = (int) (pos >> chunkSizePower); try { - return buffers[bi].getInt((int) (pos & chunkSizeMask)); + return guard.getInt(buffers[bi], (int) (pos & chunkSizeMask)); } catch (IndexOutOfBoundsException ioobe) { // either it's a boundary, or read past EOF, fall back: setPos(pos, bi); @@ -235,7 +229,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn public long readLong(long pos) throws IOException { final int bi = (int) (pos >> chunkSizePower); try { - return buffers[bi].getLong((int) (pos & chunkSizeMask)); + return guard.getLong(buffers[bi], (int) (pos & chunkSizeMask)); } catch (IndexOutOfBoundsException ioobe) { // either it's a boundary, or read past EOF, fall back: setPos(pos, bi); @@ -285,11 +279,6 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn final ByteBufferIndexInput clone = newCloneInstance(getFullSliceDescription(sliceDescription), newBuffers, ofs, length); clone.isClone = true; - - // register the new clone in our clone list to clean it up on closing: - if (clones != null) { - this.clones.put(clone, Boolean.TRUE); - } return clone; } @@ -299,9 +288,9 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn protected ByteBufferIndexInput newCloneInstance(String newResourceDescription, ByteBuffer[] newBuffers, int offset, long length) { if (newBuffers.length == 1) { newBuffers[0].position(offset); - return new SingleBufferImpl(newResourceDescription, newBuffers[0].slice(), length, chunkSizePower, this.cleaner, this.clones); + return new SingleBufferImpl(newResourceDescription, newBuffers[0].slice(), length, chunkSizePower, this.guard); } else { - return new MultiBufferImpl(newResourceDescription, newBuffers, offset, length, chunkSizePower, cleaner, clones); + return new MultiBufferImpl(newResourceDescription, newBuffers, offset, length, chunkSizePower, guard); } } @@ -335,25 +324,11 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn // make local copy, then un-set early final ByteBuffer[] bufs = buffers; unsetBuffers(); - if (clones != null) { - clones.remove(this); - } if (isClone) return; - // for extra safety unset also all clones' buffers: - if (clones != null) { - for (Iterator it = this.clones.keyIterator(); it.hasNext();) { - final ByteBufferIndexInput clone = it.next(); - assert clone.isClone; - clone.unsetBuffers(); - } - this.clones.clear(); - } - - for (final ByteBuffer b : bufs) { - freeBuffer(b); - } + // tell the guard to invalidate and later unmap the bytebuffers (if supported): + guard.invalidateAndUnmap(bufs); } finally { unsetBuffers(); } @@ -367,31 +342,12 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn curBuf = null; curBufIndex = 0; } - - /** - * Called when the contents of a buffer will be no longer needed. - */ - private void freeBuffer(ByteBuffer b) throws IOException { - if (cleaner != null) { - cleaner.freeBuffer(this, b); - } - } - - /** - * Pass in an implementation of this interface to cleanup ByteBuffers. - * MMapDirectory implements this to allow unmapping of bytebuffers with private Java APIs. - */ - @FunctionalInterface - static interface BufferCleaner { - void freeBuffer(ByteBufferIndexInput parent, ByteBuffer b) throws IOException; - } /** Optimization of ByteBufferIndexInput for when there is only one buffer */ static final class SingleBufferImpl extends ByteBufferIndexInput { - SingleBufferImpl(String resourceDescription, ByteBuffer buffer, long length, int chunkSizePower, - BufferCleaner cleaner, WeakIdentityMap clones) { - super(resourceDescription, new ByteBuffer[] { buffer }, length, chunkSizePower, cleaner, clones); + SingleBufferImpl(String resourceDescription, ByteBuffer buffer, long length, int chunkSizePower, ByteBufferGuard guard) { + super(resourceDescription, new ByteBuffer[] { buffer }, length, chunkSizePower, guard); this.curBufIndex = 0; this.curBuf = buffer; buffer.position(0); @@ -426,7 +382,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public byte readByte(long pos) throws IOException { try { - return curBuf.get((int) pos); + return guard.getByte(curBuf, (int) pos); } catch (IllegalArgumentException e) { if (pos < 0) { throw new IllegalArgumentException("Seeking to negative position: " + this, e); @@ -441,7 +397,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public short readShort(long pos) throws IOException { try { - return curBuf.getShort((int) pos); + return guard.getShort(curBuf, (int) pos); } catch (IllegalArgumentException e) { if (pos < 0) { throw new IllegalArgumentException("Seeking to negative position: " + this, e); @@ -456,7 +412,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public int readInt(long pos) throws IOException { try { - return curBuf.getInt((int) pos); + return guard.getInt(curBuf, (int) pos); } catch (IllegalArgumentException e) { if (pos < 0) { throw new IllegalArgumentException("Seeking to negative position: " + this, e); @@ -471,7 +427,7 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn @Override public long readLong(long pos) throws IOException { try { - return curBuf.getLong((int) pos); + return guard.getLong(curBuf, (int) pos); } catch (IllegalArgumentException e) { if (pos < 0) { throw new IllegalArgumentException("Seeking to negative position: " + this, e); @@ -489,8 +445,8 @@ abstract class ByteBufferIndexInput extends IndexInput implements RandomAccessIn private final int offset; MultiBufferImpl(String resourceDescription, ByteBuffer[] buffers, int offset, long length, int chunkSizePower, - BufferCleaner cleaner, WeakIdentityMap clones) { - super(resourceDescription, buffers, length, chunkSizePower, cleaner, clones); + ByteBufferGuard guard) { + super(resourceDescription, buffers, length, chunkSizePower, guard); this.offset = offset; try { seek(0L); diff --git a/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java b/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java index 60ca103a047..c0e35197f0e 100644 --- a/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java +++ b/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java @@ -36,7 +36,7 @@ import java.util.concurrent.Future; import java.lang.invoke.MethodHandle; import java.lang.reflect.Method; -import org.apache.lucene.store.ByteBufferIndexInput.BufferCleaner; +import org.apache.lucene.store.ByteBufferGuard.BufferCleaner; import org.apache.lucene.util.Constants; import org.apache.lucene.util.SuppressForbidden; @@ -240,7 +240,7 @@ public class MMapDirectory extends FSDirectory { final boolean useUnmap = getUseUnmap(); return ByteBufferIndexInput.newInstance(resourceDescription, map(resourceDescription, c, 0, c.size()), - c.size(), chunkSizePower, useUnmap ? CLEANER : null, useUnmap); + c.size(), chunkSizePower, new ByteBufferGuard(resourceDescription, useUnmap ? CLEANER : null)); } } @@ -370,7 +370,7 @@ public class MMapDirectory extends FSDirectory { final MethodHandle unmapper = filterReturnValue(directBufferCleanerMethod, guardWithTest(nonNullTest, cleanMethod, noop)) .asType(methodType(void.class, ByteBuffer.class)); - return (BufferCleaner) (ByteBufferIndexInput parent, ByteBuffer buffer) -> { + return (BufferCleaner) (String resourceDescription, ByteBuffer buffer) -> { if (directBufferClass.isInstance(buffer)) { final Throwable error = AccessController.doPrivileged((PrivilegedAction) () -> { try { @@ -381,7 +381,7 @@ public class MMapDirectory extends FSDirectory { } }); if (error != null) { - throw new IOException("Unable to unmap the mapped buffer: " + parent.toString(), error); + throw new IOException("Unable to unmap the mapped buffer: " + resourceDescription, error); } } }; diff --git a/lucene/core/src/test/org/apache/lucene/store/TestMmapDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestMmapDirectory.java index 153cc5e6d30..b87a21bc9c9 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestMmapDirectory.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestMmapDirectory.java @@ -19,6 +19,9 @@ package org.apache.lucene.store; import java.io.IOException; import java.nio.file.Path; +import java.util.concurrent.CountDownLatch; + +// import org.junit.Ignore; /** * Tests MMapDirectory @@ -39,4 +42,37 @@ public class TestMmapDirectory extends BaseDirectoryTestCase { MMapDirectory.UNMAP_SUPPORTED); } + // TODO: @Ignore("This test is for JVM testing purposes. There are no guarantees that it may not fail with SIGSEGV!") + public void testAceWithThreads() throws Exception { + for (int iter = 0; iter < 10; iter++) { + Directory dir = getDirectory(createTempDir("testAceWithThreads")); + IndexOutput out = dir.createOutput("test", IOContext.DEFAULT); + for (int i = 0; i < 8 * 1024 * 1024; i++) { + out.writeInt(random().nextInt()); + } + out.close(); + IndexInput in = dir.openInput("test", IOContext.DEFAULT); + IndexInput clone = in.clone(); + final byte accum[] = new byte[32 * 1024 * 1024]; + final CountDownLatch shotgun = new CountDownLatch(1); + Thread t1 = new Thread(() -> { + try { + shotgun.await(); + for (int i = 0; i < 10; i++) { + clone.seek(0); + clone.readBytes(accum, 0, accum.length); + } + } catch (IOException | AlreadyClosedException ok) { + // OK + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + }); + t1.start(); + shotgun.countDown(); + in.close(); + t1.join(); + dir.close(); + } + } } From f485d29cadef1cbaafd9b69d205ffe73484d9dc9 Mon Sep 17 00:00:00 2001 From: Mike McCandless Date: Fri, 12 Aug 2016 16:01:13 -0400 Subject: [PATCH 11/48] don't do random() per byte in this test --- .../src/test/org/apache/lucene/store/TestMmapDirectory.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lucene/core/src/test/org/apache/lucene/store/TestMmapDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestMmapDirectory.java index b87a21bc9c9..5790e053b2f 100644 --- a/lucene/core/src/test/org/apache/lucene/store/TestMmapDirectory.java +++ b/lucene/core/src/test/org/apache/lucene/store/TestMmapDirectory.java @@ -19,6 +19,7 @@ package org.apache.lucene.store; import java.io.IOException; import java.nio.file.Path; +import java.util.Random; import java.util.concurrent.CountDownLatch; // import org.junit.Ignore; @@ -47,8 +48,9 @@ public class TestMmapDirectory extends BaseDirectoryTestCase { for (int iter = 0; iter < 10; iter++) { Directory dir = getDirectory(createTempDir("testAceWithThreads")); IndexOutput out = dir.createOutput("test", IOContext.DEFAULT); + Random random = random(); for (int i = 0; i < 8 * 1024 * 1024; i++) { - out.writeInt(random().nextInt()); + out.writeInt(random.nextInt()); } out.close(); IndexInput in = dir.openInput("test", IOContext.DEFAULT); From 97f6bb7d7ff43f4501492eee07e6a9200b402b13 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Fri, 12 Aug 2016 22:16:04 +0200 Subject: [PATCH 12/48] LUCENE-7409: Fix comments as suggested by Dawid --- .../apache/lucene/store/ByteBufferGuard.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java b/lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java index 2e7ce2650c1..95fa17d5ea0 100644 --- a/lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java +++ b/lucene/core/src/java/org/apache/lucene/store/ByteBufferGuard.java @@ -26,10 +26,8 @@ import java.util.concurrent.atomic.AtomicInteger; * of this is used for the original and all clones, so once the original is closed and unmapped * all clones also throw {@link AlreadyClosedException}, triggered by a {@link NullPointerException}. *

- * This code uses the trick that is also used in - * {@link java.lang.invoke.MutableCallSite#syncAll(java.lang.invoke.MutableCallSite[])} to - * invalidate switch points. It also yields the current thread to give other threads a chance - * to finish in-flight requests... + * This code tries to hopefully flush any CPU caches using a store-store barrier. It also yields the + * current thread to give other threads a chance to finish in-flight requests... */ final class ByteBufferGuard { @@ -45,10 +43,10 @@ final class ByteBufferGuard { private final String resourceDescription; private final BufferCleaner cleaner; - /** not volatile, we use store-store barrier! */ + /** Not volatile; see comments on visibility below! */ private boolean invalidated = false; - /** the actual store-store barrier. */ + /** Used as a store-store barrier; see comments below! */ private final AtomicInteger barrier = new AtomicInteger(); /** @@ -66,9 +64,17 @@ final class ByteBufferGuard { public void invalidateAndUnmap(ByteBuffer... bufs) throws IOException { if (cleaner != null) { invalidated = true; - // this should trigger a happens-before - so flushes all caches + // This call should hopefully flush any CPU caches and as a result make + // the "invalidated" field update visible to other threads. We specifically + // don't make "invalidated" field volatile for performance reasons, hoping the + // JVM won't optimize away reads of that field and hardware should ensure + // caches are in sync after this call. This isn't entirely "fool-proof" + // (see LUCENE-7409 discussion), but it has been shown to work in practice + // and we count on this behavior. barrier.lazySet(0); + // we give other threads a bit of time to finish reads on their ByteBuffer...: Thread.yield(); + // finally unmap the ByteBuffers: for (ByteBuffer b : bufs) { cleaner.freeBuffer(resourceDescription, b); } From 70d27aec83f9257da459f157acd9fc70764f7195 Mon Sep 17 00:00:00 2001 From: Varun Thacker Date: Sat, 13 Aug 2016 16:20:51 +0530 Subject: [PATCH 13/48] SOLR-9320: Improve logging --- .../org/apache/solr/cloud/DeleteNodeCmd.java | 22 +++++++----- .../OverseerCollectionMessageHandler.java | 6 ++-- .../org/apache/solr/cloud/ReplaceNodeCmd.java | 35 +++++++++---------- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/DeleteNodeCmd.java b/solr/core/src/java/org/apache/solr/cloud/DeleteNodeCmd.java index cbcfa8847df..3e600908cd6 100644 --- a/solr/core/src/java/org/apache/solr/cloud/DeleteNodeCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/DeleteNodeCmd.java @@ -20,6 +20,7 @@ package org.apache.solr.cloud; import java.lang.invoke.MethodHandles; import java.util.List; +import java.util.Locale; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -27,11 +28,13 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.Utils; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP; +import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP; + public class DeleteNodeCmd implements OverseerCollectionMessageHandler.Cmd { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -42,43 +45,44 @@ public class DeleteNodeCmd implements OverseerCollectionMessageHandler.Cmd { } @Override - public Object call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception { + public void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception { ocmh.checkRequired(message, "node"); String node = message.getStr("node"); if (!state.liveNodesContain(node)) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Source Node: " + node + " is not live"); } List sourceReplicas = ReplaceNodeCmd.getReplicasOfNode(node, state); - cleanupReplicas(results, state, sourceReplicas, ocmh); - return null; + cleanupReplicas(results, state, sourceReplicas, ocmh, node); } static void cleanupReplicas(NamedList results, ClusterState clusterState, List sourceReplicas, - OverseerCollectionMessageHandler ocmh) throws InterruptedException { + OverseerCollectionMessageHandler ocmh, String node) throws InterruptedException { CountDownLatch cleanupLatch = new CountDownLatch(sourceReplicas.size()); for (ZkNodeProps sourceReplica : sourceReplicas) { - log.info("deleting replica from from node {} ", Utils.toJSONString(sourceReplica)); + log.info("Deleting replica for collection={} shard={} on node={}", sourceReplica.getStr(COLLECTION_PROP), sourceReplica.getStr(SHARD_ID_PROP), node); NamedList deleteResult = new NamedList(); try { ocmh.deleteReplica(clusterState, sourceReplica.plus("parallel", "true"), deleteResult, () -> { cleanupLatch.countDown(); if (deleteResult.get("failure") != null) { synchronized (results) { - results.add("failure", "could not delete because " + deleteResult.get("failure") + " " + Utils.toJSONString(sourceReplica)); + results.add("failure", String.format(Locale.ROOT, "Failed to delete replica for collection=%s shard=%s" + + " on node=%s", sourceReplica.getStr(COLLECTION_PROP), sourceReplica.getStr(SHARD_ID_PROP), node)); } } }); } catch (KeeperException e) { - log.info("Error deleting ", e); + log.warn("Error deleting ", e); cleanupLatch.countDown(); } catch (Exception e) { + log.warn("Error deleting ", e); cleanupLatch.countDown(); throw e; } } - log.info("Waiting for deletes to complete"); + log.debug("Waiting for delete node action to complete"); cleanupLatch.await(5, TimeUnit.MINUTES); } diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java index 908d35c3e6b..0588446c653 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java @@ -316,7 +316,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler default: { Cmd command = commandMap.get(action); if (command != null) { - command.call(zkStateReader.getClusterState(),message, results); + command.call(zkStateReader.getClusterState(), message, results); } else { throw new SolrException(ErrorCode.BAD_REQUEST, "Unknown operation:" + operation); @@ -617,7 +617,6 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler @SuppressWarnings("unchecked") void deleteReplica(ClusterState clusterState, ZkNodeProps message, NamedList results, Runnable onComplete) throws KeeperException, InterruptedException { - log.info("deleteReplica() : {}", Utils.toJSONString(message)); checkRequired(message, COLLECTION_PROP, SHARD_ID_PROP, REPLICA_PROP); String collectionName = message.getStr(COLLECTION_PROP); String shard = message.getStr(SHARD_ID_PROP); @@ -664,7 +663,6 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler params.set(CoreAdminParams.DELETE_DATA_DIR, message.getBool(CoreAdminParams.DELETE_DATA_DIR, true)); sendShardRequest(replica.getNodeName(), params, shardHandler, asyncId, requestMap.get()); - AtomicReference exp = new AtomicReference<>(); Callable callable = () -> { try { @@ -2809,7 +2807,7 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler interface Cmd { - Object call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception; + void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception; } diff --git a/solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java b/solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java index 0cfd0894ffe..aad9cc721fb 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java @@ -21,6 +21,7 @@ package org.apache.solr.cloud; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -35,7 +36,6 @@ import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.util.NamedList; -import org.apache.solr.common.util.Utils; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,7 +53,7 @@ public class ReplaceNodeCmd implements OverseerCollectionMessageHandler.Cmd { } @Override - public Object call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception { + public void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception { ZkStateReader zkStateReader = ocmh.zkStateReader; ocmh.checkRequired(message, "source", "target"); String source = message.getStr("source"); @@ -76,23 +76,25 @@ public class ReplaceNodeCmd implements OverseerCollectionMessageHandler.Cmd { for (ZkNodeProps sourceReplica : sourceReplicas) { NamedList nl = new NamedList(); - log.info("going to create replica {}", Utils.toJSONString(sourceReplica)); + log.info("Going to create replica for collection={} shard={} on node={}", sourceReplica.getStr(COLLECTION_PROP), sourceReplica.getStr(SHARD_ID_PROP), target); ZkNodeProps msg = sourceReplica.plus("parallel", String.valueOf(parallel)).plus(CoreAdminParams.NODE, target); final ZkNodeProps addedReplica = ocmh.addReplica(clusterState, msg, nl, () -> { countDownLatch.countDown(); if (nl.get("failure") != null) { - log.warn("failed to create : " + Utils.toJSONString(msg)); + String errorString = String.format(Locale.ROOT, "Failed to create replica for collection=%s shard=%s" + + " on node=%s", sourceReplica.getStr(COLLECTION_PROP), sourceReplica.getStr(SHARD_ID_PROP), target); + log.warn(errorString); // one replica creation failed. Make the best attempt to // delete all the replicas created so far in the target // and exit synchronized (results) { - results.add("failure", "Could not create copy of replica " + Utils.toJSONString(sourceReplica)); + results.add("failure", errorString); anyOneFailed.set(true); } } else { - log.info("successfully created : " + Utils.toJSONString(msg)); - + log.debug("Successfully created replica for collection={} shard={} on node={}", + sourceReplica.getStr(COLLECTION_PROP), sourceReplica.getStr(SHARD_ID_PROP), target); } }); @@ -101,12 +103,12 @@ public class ReplaceNodeCmd implements OverseerCollectionMessageHandler.Cmd { } } - log.info("Waiting for creates to complete "); + log.debug("Waiting for replace node action to complete"); countDownLatch.await(5, TimeUnit.MINUTES); - log.info("Waiting over for creates to complete "); + log.debug("Finished waiting for replace node action to complete"); if (anyOneFailed.get()) { - log.info("failed to create some cores delete all " + Utils.toJSONString(createdReplicas)); + log.info("Failed to create some replicas. Cleaning up all replicas on target node"); CountDownLatch cleanupLatch = new CountDownLatch(createdReplicas.size()); for (ZkNodeProps createdReplica : createdReplicas) { NamedList deleteResult = new NamedList(); @@ -115,29 +117,27 @@ public class ReplaceNodeCmd implements OverseerCollectionMessageHandler.Cmd { cleanupLatch.countDown(); if (deleteResult.get("failure") != null) { synchronized (results) { - results.add("failure", "could not cleanup, because : " + deleteResult.get("failure") + " " + Utils.toJSONString(createdReplica)); + results.add("failure", "Could not cleanup, because of : " + deleteResult.get("failure")); } } }); } catch (KeeperException e) { cleanupLatch.countDown(); - log.info("Error deleting ", e); + log.warn("Error deleting replica ", e); } catch (Exception e) { - log.error("Unknown Error deleteing", e); + log.warn("Error deleting replica ", e); cleanupLatch.countDown(); throw e; } } cleanupLatch.await(5, TimeUnit.MINUTES); - return null; } // we have reached this far means all replicas could be recreated //now cleanup the replicas in the source node - DeleteNodeCmd.cleanupReplicas(results, state, sourceReplicas, ocmh); - results.add("success", "REPLACENODE completed successfully from : " + source + " to : " + target); - return null; + DeleteNodeCmd.cleanupReplicas(results, state, sourceReplicas, ocmh, source); + results.add("success", "REPLACENODE action completed successfully from : " + source + " to : " + target); } static List getReplicasOfNode(String source, ClusterState state) { @@ -152,7 +152,6 @@ public class ReplaceNodeCmd implements OverseerCollectionMessageHandler.Cmd { ZkStateReader.CORE_NAME_PROP, replica.getCoreName(), ZkStateReader.REPLICA_PROP, replica.getName(), CoreAdminParams.NODE, source); - log.info("src_core : {}", Utils.toJSONString(props)); sourceReplicas.add(props ); } From 1d9be84cb67ed5e57bcd60ae483f45d3abd09bd5 Mon Sep 17 00:00:00 2001 From: Varun Thacker Date: Sat, 13 Aug 2016 16:52:47 +0530 Subject: [PATCH 14/48] SOLR-9092: In the deletereplica commandand add a live check before calling delete core --- solr/CHANGES.txt | 3 +++ .../cloud/OverseerCollectionMessageHandler.java | 13 +++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 5d4e2a64bf7..eddb7fba206 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -267,6 +267,9 @@ Other Changes * SOLR-9331: Remove ReRankQuery's length constructor argument and member. (Christine Poerschke) +* SOLR-9092: For the delete replica command we attempt to send the core admin delete request only + if that node is actually up. (Jessica Cheng Mallet, Varun Thacker) + ================== 6.1.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java index 0588446c653..49e094284b3 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java @@ -662,14 +662,19 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler params.set(CoreAdminParams.DELETE_INSTANCE_DIR, message.getBool(CoreAdminParams.DELETE_INSTANCE_DIR, true)); params.set(CoreAdminParams.DELETE_DATA_DIR, message.getBool(CoreAdminParams.DELETE_DATA_DIR, true)); - sendShardRequest(replica.getNodeName(), params, shardHandler, asyncId, requestMap.get()); + boolean isLive = zkStateReader.getClusterState().getLiveNodes().contains(replica.getNodeName()); + if (isLive) { + sendShardRequest(replica.getNodeName(), params, shardHandler, asyncId, requestMap.get()); + } Callable callable = () -> { try { - processResponses(results, shardHandler, false, null, asyncId, requestMap.get()); + if (isLive) { + processResponses(results, shardHandler, false, null, asyncId, requestMap.get()); - //check if the core unload removed the corenode zk entry - if (waitForCoreNodeGone(collectionName, shard, replicaName, 5000)) return Boolean.TRUE; + //check if the core unload removed the corenode zk entry + if (waitForCoreNodeGone(collectionName, shard, replicaName, 5000)) return Boolean.TRUE; + } // try and ensure core info is removed from cluster state deleteCoreNode(collectionName, replicaName, replica, core); From c9faa102f99d7e19df5bcd63e16e699f52f2b1db Mon Sep 17 00:00:00 2001 From: Alexandre Rafalovitch Date: Sun, 14 Aug 2016 00:28:40 +1000 Subject: [PATCH 15/48] SOLR-8911: Enable scrolling in Admin UI overflows In the dashboard screen, scroll horizontally the Versions and JVM property values strings. --- solr/CHANGES.txt | 1 + solr/webapp/web/css/angular/index.css | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index eddb7fba206..cf934481af9 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -210,6 +210,7 @@ Bug Fixes * SOLR-8715: Admin UI's Schema screen now works for fields with stored=false and some content indexed (Alexandre Rafalovitch) +* SOLR-8911: In Admin UI, enable scrolling for overflowing Versions and JVM property values Optimizations ---------------------- diff --git a/solr/webapp/web/css/angular/index.css b/solr/webapp/web/css/angular/index.css index 5b77a15a866..e07b8d62686 100644 --- a/solr/webapp/web/css/angular/index.css +++ b/solr/webapp/web/css/angular/index.css @@ -110,6 +110,17 @@ limitations under the License. width: 40%; } +#content #index .data +{ + padding-bottom: 12px; + overflow: hidden; +} + +#content #index .data:hover +{ + overflow-x: auto; +} + #content #index .data li { padding-top: 3px; @@ -127,7 +138,6 @@ limitations under the License. { float: right; text-overflow: ellipsis; - overflow: hidden; white-space: nowrap; width: 80% } From 97dc5a2a0bd2a00e227cb7b6621f827f64b01457 Mon Sep 17 00:00:00 2001 From: Alexandre Rafalovitch Date: Sun, 14 Aug 2016 08:03:43 +1000 Subject: [PATCH 16/48] SOLR-9002: Fix type mapping for JSON and text This is for the Admin UI's collection/File screen --- solr/CHANGES.txt | 3 +++ solr/webapp/web/js/angular/controllers/files.js | 6 +++--- solr/webapp/web/js/angular/services.js | 2 ++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index cf934481af9..3c54bad0d76 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -211,6 +211,9 @@ Bug Fixes * SOLR-8715: Admin UI's Schema screen now works for fields with stored=false and some content indexed (Alexandre Rafalovitch) * SOLR-8911: In Admin UI, enable scrolling for overflowing Versions and JVM property values + +* SOLR-9002: Admin UI now correctly displays json and text files in the collection/Files screen + Optimizations ---------------------- diff --git a/solr/webapp/web/js/angular/controllers/files.js b/solr/webapp/web/js/angular/controllers/files.js index 1cb9e5cd55e..00ea4b19ff7 100644 --- a/solr/webapp/web/js/angular/controllers/files.js +++ b/solr/webapp/web/js/angular/controllers/files.js @@ -16,7 +16,7 @@ */ var contentTypeMap = { xml : 'text/xml', html : 'text/html', js : 'text/javascript', json : 'application/json', 'css' : 'text/css' }; -var languages = {js: "javascript", xml:"xml", xsl:"xml", vm: "xml", html: "xml", json: "text", css: "css"}; +var languages = {js: "javascript", xml:"xml", xsl:"xml", vm: "xml", html: "xml", json: "json", css: "css"}; solrAdminApp.controller('FilesController', function($scope, $rootScope, $routeParams, $location, Files, Constants) { @@ -82,10 +82,10 @@ solrAdminApp.controller('FilesController', Files.get({core: $routeParams.core, file: $scope.file, contentType: contentType}, function(data) { $scope.content = data.data; $scope.url = $scope.baseurl + data.config.url + "?" + $.param(data.config.params); - if (contentType.indexOf("text/plain") && data.data.indexOf("=0) || data.data.indexOf("