You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2013/07/30 16:51:57 UTC
svn commit: r1508476 - in /lucene/dev/trunk/solr/core/src:
java/org/apache/solr/handler/admin/ java/org/apache/solr/update/
test/org/apache/solr/handler/admin/
Author: erick
Date: Tue Jul 30 14:51:56 2013
New Revision: 1508476
URL: http://svn.apache.org/r1508476
Log:
SOLR-5087, CoreAdminHandler.handleMergeAction generating NullPointerException. Thanks Patrick
Added:
lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/admin/CoreMergeIndexesAdminHandlerTest.java (with props)
Modified:
lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/MergeIndexesCommand.java
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java?rev=1508476&r1=1508475&r2=1508476&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java Tue Jul 30 14:51:56 2013
@@ -18,6 +18,7 @@
package org.apache.solr.handler.admin;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.lucene.index.DirectoryReader;
@@ -295,17 +296,17 @@ public class CoreAdminHandler extends Re
}
- protected void handleMergeAction(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException {
+ protected void handleMergeAction(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
SolrParams params = req.getParams();
String cname = params.required().get(CoreAdminParams.CORE);
SolrCore core = coreContainer.getCore(cname);
SolrQueryRequest wrappedReq = null;
- SolrCore[] sourceCores = null;
- RefCounted<SolrIndexSearcher>[] searchers = null;
+ List<SolrCore> sourceCores = Lists.newArrayList();
+ List<RefCounted<SolrIndexSearcher>> searchers = Lists.newArrayList();
// stores readers created from indexDir param values
- DirectoryReader[] readersToBeClosed = null;
- Directory[] dirsToBeReleased = null;
+ List<DirectoryReader> readersToBeClosed = Lists.newArrayList();
+ List<Directory> dirsToBeReleased = Lists.newArrayList();
if (core != null) {
try {
String[] dirNames = params.getParams(CoreAdminParams.INDEX_DIR);
@@ -315,38 +316,34 @@ public class CoreAdminHandler extends Re
throw new SolrException( SolrException.ErrorCode.BAD_REQUEST,
"At least one indexDir or srcCore must be specified");
- sourceCores = new SolrCore[sources.length];
for (int i = 0; i < sources.length; i++) {
String source = sources[i];
SolrCore srcCore = coreContainer.getCore(source);
if (srcCore == null)
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"Core: " + source + " does not exist");
- sourceCores[i] = srcCore;
+ sourceCores.add(srcCore);
}
} else {
- readersToBeClosed = new DirectoryReader[dirNames.length];
- dirsToBeReleased = new Directory[dirNames.length];
DirectoryFactory dirFactory = core.getDirectoryFactory();
for (int i = 0; i < dirNames.length; i++) {
Directory dir = dirFactory.get(dirNames[i], DirContext.DEFAULT, core.getSolrConfig().indexConfig.lockType);
- dirsToBeReleased[i] = dir;
+ dirsToBeReleased.add(dir);
// TODO: why doesn't this use the IR factory? what is going on here?
- readersToBeClosed[i] = DirectoryReader.open(dir);
+ readersToBeClosed.add(DirectoryReader.open(dir));
}
}
- DirectoryReader[] readers = null;
- if (readersToBeClosed != null) {
+ List<DirectoryReader> readers = null;
+ if (readersToBeClosed.size() > 0) {
readers = readersToBeClosed;
} else {
- readers = new DirectoryReader[sourceCores.length];
- searchers = new RefCounted[sourceCores.length];
- for (int i = 0; i < sourceCores.length; i++) {
- SolrCore solrCore = sourceCores[i];
+ readers = Lists.newArrayList();
+ for (SolrCore solrCore: sourceCores) {
// record the searchers so that we can decref
- searchers[i] = solrCore.getSearcher();
- readers[i] = searchers[i].get().getIndexReader();
+ RefCounted<SolrIndexSearcher> searcher = solrCore.getSearcher();
+ searchers.add(searcher);
+ readers.add(searcher.get().getIndexReader());
}
}
@@ -356,23 +353,21 @@ public class CoreAdminHandler extends Re
UpdateRequestProcessor processor =
processorChain.createProcessor(wrappedReq, rsp);
processor.processMergeIndexes(new MergeIndexesCommand(readers, req));
+ } catch (Exception e) {
+ // log and rethrow so that if the finally fails we don't lose the original problem
+ log.error("ERROR executing merge:", e);
+ throw e;
} finally {
- if (searchers != null) {
- for (RefCounted<SolrIndexSearcher> searcher : searchers) {
- if (searcher != null) searcher.decref();
- }
+ for (RefCounted<SolrIndexSearcher> searcher : searchers) {
+ if (searcher != null) searcher.decref();
}
- if (sourceCores != null) {
- for (SolrCore solrCore : sourceCores) {
- if (solrCore != null) solrCore.close();
- }
+ for (SolrCore solrCore : sourceCores) {
+ if (solrCore != null) solrCore.close();
}
- if (readersToBeClosed != null) IOUtils.closeWhileHandlingException(readersToBeClosed);
- if (dirsToBeReleased != null) {
- for (Directory dir : dirsToBeReleased) {
- DirectoryFactory dirFactory = core.getDirectoryFactory();
- dirFactory.release(dir);
- }
+ IOUtils.closeWhileHandlingException(readersToBeClosed);
+ for (Directory dir : dirsToBeReleased) {
+ DirectoryFactory dirFactory = core.getDirectoryFactory();
+ dirFactory.release(dir);
}
if (wrappedReq != null) wrappedReq.close();
core.close();
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java?rev=1508476&r1=1508475&r2=1508476&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java Tue Jul 30 14:51:56 2013
@@ -31,6 +31,7 @@ import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicLong;
import org.apache.lucene.document.Document;
+import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.Term;
@@ -432,11 +433,11 @@ public class DirectUpdateHandler2 extend
log.info("start " + cmd);
- IndexReader[] readers = cmd.readers;
- if (readers != null && readers.length > 0) {
+ List<DirectoryReader> readers = cmd.readers;
+ if (readers != null && readers.size() > 0) {
RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);
try {
- iw.get().addIndexes(readers);
+ iw.get().addIndexes(readers.toArray(new IndexReader[readers.size()]));
} finally {
iw.decref();
}
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/MergeIndexesCommand.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/MergeIndexesCommand.java?rev=1508476&r1=1508475&r2=1508476&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/MergeIndexesCommand.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/MergeIndexesCommand.java Tue Jul 30 14:51:56 2013
@@ -17,9 +17,14 @@
package org.apache.solr.update;
+import com.google.common.base.Function;
+import com.google.common.base.Joiner;
+import com.google.common.collect.Iterables;
import org.apache.lucene.index.DirectoryReader;
import org.apache.solr.request.SolrQueryRequest;
+import java.util.List;
+
/**
* A merge indexes command encapsulated in an object.
*
@@ -27,9 +32,9 @@ import org.apache.solr.request.SolrQuery
*
*/
public class MergeIndexesCommand extends UpdateCommand {
- public DirectoryReader[] readers;
+ public List<DirectoryReader> readers;
- public MergeIndexesCommand(DirectoryReader[] readers, SolrQueryRequest req) {
+ public MergeIndexesCommand(List<DirectoryReader> readers, SolrQueryRequest req) {
super(req);
this.readers = readers;
}
@@ -42,12 +47,13 @@ public class MergeIndexesCommand extends
@Override
public String toString() {
StringBuilder sb = new StringBuilder(super.toString());
- if (readers != null && readers.length > 0) {
- sb.append(readers[0].directory());
- for (int i = 1; i < readers.length; i++) {
- sb.append(",").append(readers[i].directory());
+ Joiner joiner = Joiner.on(",");
+ Iterable<String> directories = Iterables.transform(readers, new Function<DirectoryReader, String>() {
+ public String apply(DirectoryReader reader) {
+ return reader.directory().toString();
}
- }
+ });
+ joiner.skipNulls().join(sb, directories);
sb.append('}');
return sb.toString();
}
Added: lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/admin/CoreMergeIndexesAdminHandlerTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/admin/CoreMergeIndexesAdminHandlerTest.java?rev=1508476&view=auto
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/admin/CoreMergeIndexesAdminHandlerTest.java (added)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/handler/admin/CoreMergeIndexesAdminHandlerTest.java Tue Jul 30 14:51:56 2013
@@ -0,0 +1,112 @@
+package org.apache.solr.handler.admin;
+
+/*
+ * 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.
+ */
+
+import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
+import org.apache.commons.io.FileUtils;
+import org.apache.lucene.store.Directory;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.MockFSDirectoryFactory;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TestRule;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Map;
+
+public class CoreMergeIndexesAdminHandlerTest extends SolrTestCaseJ4 {
+
+ @BeforeClass
+ public static void beforeClass() throws Exception {
+ useFactory(FailingDirectoryFactory.class.getName());
+ initCore("solrconfig.xml", "schema.xml");
+ }
+
+ @Rule
+ public TestRule solrTestRules = RuleChain.outerRule(new SystemPropertiesRestoreRule());
+
+
+ private static String FAILING_MSG = "Creating a directory using FailingDirectoryFactoryException always fails";
+ public static class FailingDirectoryFactory extends MockFSDirectoryFactory {
+ public class FailingDirectoryFactoryException extends RuntimeException {
+ public FailingDirectoryFactoryException() {
+ super(FAILING_MSG);
+ }
+ }
+
+ public boolean fail = false;
+ @Override
+ public Directory create(String path, DirContext dirContext) throws IOException {
+ if (fail) {
+ throw new FailingDirectoryFactoryException();
+ } else {
+ return super.create(path, dirContext);
+ }
+ }
+ }
+
+ @Test
+ public void testMergeIndexesCoreAdminHandler() throws Exception {
+ final File workDir = new File(TEMP_DIR, this.getClass().getName());
+
+ if (workDir.exists()) {
+ FileUtils.deleteDirectory(workDir);
+ }
+ assertTrue("Failed to mkdirs workDir", workDir.mkdirs());
+
+ final CoreContainer cores = h.getCoreContainer();
+
+ final CoreAdminHandler admin = new CoreAdminHandler(cores);
+
+ SolrCore core = cores.getCore("collection1");
+ try {
+ FailingDirectoryFactory dirFactory = (FailingDirectoryFactory)core.getDirectoryFactory();
+
+ try {
+ dirFactory.fail = true;
+ ignoreException(FAILING_MSG);
+
+ SolrQueryResponse resp = new SolrQueryResponse();
+ admin.handleRequestBody
+ (req(CoreAdminParams.ACTION,
+ CoreAdminParams.CoreAdminAction.MERGEINDEXES.toString(),
+ CoreAdminParams.CORE, "collection1",
+ CoreAdminParams.INDEX_DIR, workDir.getAbsolutePath()),
+ resp);
+ fail("exception expected");
+ } catch (FailingDirectoryFactory.FailingDirectoryFactoryException e) {
+ // expected if error handling properly
+ } finally {
+ unIgnoreException(FAILING_MSG);
+ }
+ dirFactory.fail = false;
+ } finally {
+ core.close();
+ }
+
+ // cleanup
+ FileUtils.deleteDirectory(workDir);
+ }
+}