SOLR-10054: Core swapping doesn't work with new metrics changes in place.

This commit is contained in:
Andrzej Bialecki 2017-02-01 10:20:08 +01:00
parent bbc455de19
commit bef725aeef
9 changed files with 208 additions and 55 deletions

View File

@ -175,10 +175,13 @@ Jetty 9.3.14.v20161028
Bug Fixes
----------------------
* SOLR-9969: "Plugin/Stats" section of the UI doesn't display empty metric types (Tomás Fernández Löbbe)
* SOLR-8491: solr.cmd SOLR_SSL_OPTS is overwritten (Sam Yi, Andy Hind, Marcel Berteler, Kevin Risden)
* SOLR-10031: Validation of filename params in ReplicationHandler (Hrishikesh Gadre, janhoy)
* SOLR-10054: Core swapping doesn't work with new metrics changes in place (ab)
================== 6.4.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

View File

@ -77,6 +77,7 @@ import org.apache.solr.handler.admin.ZookeeperInfoHandler;
import org.apache.solr.handler.component.ShardHandlerFactory;
import org.apache.solr.logging.LogWatcher;
import org.apache.solr.logging.MDCLoggingContext;
import org.apache.solr.metrics.SolrCoreMetricManager;
import org.apache.solr.metrics.SolrMetricManager;
import org.apache.solr.metrics.SolrMetricProducer;
import org.apache.solr.request.SolrRequestHandler;
@ -1183,15 +1184,15 @@ public class CoreContainer {
SolrCore core = solrCores.remove(name);
coresLocator.delete(this, cd);
// delete metrics specific to this core
metricManager.removeRegistry(SolrMetricManager.getRegistryName(SolrInfoMBean.Group.core, name));
if (core == null) {
// transient core
SolrCore.deleteUnloadedCore(cd, deleteDataDir, deleteInstanceDir);
return;
}
// delete metrics specific to this core
metricManager.removeRegistry(core.getCoreMetricManager().getRegistryName());
if (zkSys.getZkController() != null) {
// cancel recovery in cloud mode
core.getSolrCoreState().cancelRecovery();
@ -1217,6 +1218,9 @@ public class CoreContainer {
SolrIdentifierValidator.validateCoreName(toName);
try (SolrCore core = getCore(name)) {
if (core != null) {
String oldRegistryName = core.getCoreMetricManager().getRegistryName();
String newRegistryName = SolrCoreMetricManager.createRegistryName(core.getCoreDescriptor().getCollectionName(), toName);
metricManager.swapRegistries(oldRegistryName, newRegistryName);
registerCore(toName, core, true, false);
SolrCore old = solrCores.remove(name);
coresLocator.rename(this, old.getCoreDescriptor(), core.getCoreDescriptor());

View File

@ -239,7 +239,9 @@ class SolrCores {
}
cores.put(n0, c1);
cores.put(n1, c0);
container.getMetricManager().swapRegistries(
c0.getCoreMetricManager().getRegistryName(),
c1.getCoreMetricManager().getRegistryName());
c0.setName(n1);
c1.setName(n0);
}

View File

@ -73,9 +73,6 @@ public class SolrCoreMetricManager implements Closeable {
}
// close old reporters
metricManager.closeReporters(oldRegistryName);
metricManager.moveMetrics(oldRegistryName, registryName, null);
// old registry is no longer used - we have moved the metrics
metricManager.removeRegistry(oldRegistryName);
// load reporters again, using the new core name
loadReporters();
}

View File

@ -85,6 +85,7 @@ public class SolrMetricManager {
private final Map<String, Map<String, SolrMetricReporter>> reporters = new HashMap<>();
private final Lock reportersLock = new ReentrantLock();
private final Lock swapLock = new ReentrantLock();
public SolrMetricManager() { }
@ -178,21 +179,30 @@ public class SolrMetricManager {
if (isSharedRegistry(registry)) {
return SharedMetricRegistries.getOrCreate(registry);
} else {
final MetricRegistry existing = registries.get(registry);
if (existing == null) {
final MetricRegistry created = new MetricRegistry();
final MetricRegistry raced = registries.putIfAbsent(registry, created);
if (raced == null) {
return created;
} else {
return raced;
}
} else {
return existing;
swapLock.lock();
try {
return getOrCreate(registries, registry);
} finally {
swapLock.unlock();
}
}
}
private static MetricRegistry getOrCreate(ConcurrentMap<String, MetricRegistry> map, String registry) {
final MetricRegistry existing = map.get(registry);
if (existing == null) {
final MetricRegistry created = new MetricRegistry();
final MetricRegistry raced = map.putIfAbsent(registry, created);
if (raced == null) {
return created;
} else {
return raced;
}
} else {
return existing;
}
}
/**
* Remove a named registry.
* @param registry name of the registry to remove
@ -205,34 +215,47 @@ public class SolrMetricManager {
if (isSharedRegistry(registry)) {
SharedMetricRegistries.remove(registry);
} else {
registries.remove(registry);
swapLock.lock();
try {
registries.remove(registry);
} finally {
swapLock.unlock();
}
}
}
/**
* Move all matching metrics from one registry to another. This is useful eg. during
* {@link org.apache.solr.core.SolrCore} rename or swap operations.
* @param fromRegistry source registry
* @param toRegistry target registry
* @param filter optional {@link MetricFilter} to select what metrics to move. If null
* then all metrics will be moved.
* Swap registries. This is useful eg. during
* {@link org.apache.solr.core.SolrCore} rename or swap operations. NOTE:
* this operation is not supported for shared registries.
* @param registry1 source registry
* @param registry2 target registry. Note: when used after core rename the target registry doesn't
* exist, so the swap operation will only rename the existing registry without creating
* an empty one under the previous name.
*/
public void moveMetrics(String fromRegistry, String toRegistry, MetricFilter filter) {
MetricRegistry from = registry(fromRegistry);
MetricRegistry to = registry(toRegistry);
if (from == to) {
return;
public void swapRegistries(String registry1, String registry2) {
registry1 = overridableRegistryName(registry1);
registry2 = overridableRegistryName(registry2);
if (isSharedRegistry(registry1) || isSharedRegistry(registry2)) {
throw new UnsupportedOperationException("Cannot swap shared registry: " + registry1 + ", " + registry2);
}
if (filter == null) {
to.registerAll(from);
from.removeMatching(MetricFilter.ALL);
} else {
for (Map.Entry<String, Metric> entry : from.getMetrics().entrySet()) {
if (filter.matches(entry.getKey(), entry.getValue())) {
to.register(entry.getKey(), entry.getValue());
}
swapLock.lock();
try {
MetricRegistry from = registries.get(registry1);
MetricRegistry to = registries.get(registry2);
if (from == to) {
return;
}
from.removeMatching(filter);
MetricRegistry reg1 = registries.remove(registry1);
MetricRegistry reg2 = registries.remove(registry2);
if (reg2 != null) {
registries.put(registry1, reg2);
}
if (reg1 != null) {
registries.put(registry2, reg1);
}
} finally {
swapLock.unlock();
}
}

View File

@ -51,7 +51,7 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 {
}
@Test
public void testMoveMetrics() throws Exception {
public void testSwapRegistries() throws Exception {
Random r = random();
SolrMetricManager metricManager = new SolrMetricManager();
@ -65,13 +65,14 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 {
metricManager.register(fromName, entry.getValue(), false, entry.getKey(), "metrics1");
}
for (Map.Entry<String, Counter> entry : metrics2.entrySet()) {
metricManager.register(fromName, entry.getValue(), false, entry.getKey(), "metrics2");
metricManager.register(toName, entry.getValue(), false, entry.getKey(), "metrics2");
}
assertEquals(metrics1.size() + metrics2.size(), metricManager.registry(fromName).getMetrics().size());
assertEquals(metrics1.size(), metricManager.registry(fromName).getMetrics().size());
assertEquals(metrics2.size(), metricManager.registry(toName).getMetrics().size());
// move metrics1
metricManager.moveMetrics(fromName, toName, new SolrMetricManager.PrefixFilter("metrics1"));
// check the remaining metrics
// swap
metricManager.swapRegistries(fromName, toName);
// check metrics
Map<String, Metric> fromMetrics = metricManager.registry(fromName).getMetrics();
assertEquals(metrics2.size(), fromMetrics.size());
for (Map.Entry<String, Counter> entry : metrics2.entrySet()) {
@ -79,7 +80,6 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 {
assertNotNull(value);
assertEquals(entry.getValue(), value);
}
// check the moved metrics
Map<String, Metric> toMetrics = metricManager.registry(toName).getMetrics();
assertEquals(metrics1.size(), toMetrics.size());
for (Map.Entry<String, Counter> entry : metrics1.entrySet()) {
@ -87,13 +87,6 @@ public class SolrMetricManagerTest extends SolrTestCaseJ4 {
assertNotNull(value);
assertEquals(entry.getValue(), value);
}
// move all remaining metrics
metricManager.moveMetrics(fromName, toName, null);
fromMetrics = metricManager.registry(fromName).getMetrics();
assertEquals(0, fromMetrics.size());
toMetrics = metricManager.registry(toName).getMetrics();
assertEquals(metrics1.size() + metrics2.size(), toMetrics.size());
}
@Test

View File

@ -619,6 +619,24 @@ public class CoreAdminRequest extends SolrRequest<CoreAdminResponse> {
return req.process( client );
}
/**
* Swap two existing cores.
* @param core1 name of the first core
* @param core2 name of the other core
* @param client SolrClient to use
* @return response
* @throws SolrServerException if one or both cores don't exist
* @throws IOException on IO errors
*/
public static CoreAdminResponse swapCore(String core1, String core2, SolrClient client)
throws SolrServerException, IOException {
CoreAdminRequest req = new CoreAdminRequest();
req.setCoreName(core1);
req.setOtherCoreName(core2);
req.setAction( CoreAdminAction.SWAP );
return req.process( client );
}
public static CoreStatus getCoreStatus(String coreName, SolrClient client) throws SolrServerException, IOException {
return getCoreStatus(coreName, true, client);
}

View File

@ -19,26 +19,37 @@ package org.apache.solr.client.solrj.embedded;
import java.io.File;
import java.nio.file.Path;
import org.apache.commons.io.FileUtils;
import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.core.CoreContainer;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
public abstract class AbstractEmbeddedSolrServerTestCase extends SolrTestCaseJ4 {
protected static final Path SOLR_HOME = getFile("solrj/solr/shared").toPath().toAbsolutePath();
protected static Path SOLR_HOME;
protected static Path CONFIG_HOME;
protected CoreContainer cores = null;
protected File tempDir;
@BeforeClass
public static void setUpHome() throws Exception {
CONFIG_HOME = getFile("solrj/solr/shared").toPath().toAbsolutePath();
SOLR_HOME = createTempDir("solrHome");
FileUtils.copyDirectory(CONFIG_HOME.toFile(), SOLR_HOME.toFile());
}
@Override
@Before
public void setUp() throws Exception {
super.setUp();
System.setProperty("solr.solr.home", SOLR_HOME.toString());
System.setProperty("configSetBase", SOLR_HOME.resolve("../configsets").normalize().toString());
System.setProperty("configSetBaseDir", CONFIG_HOME.resolve("../configsets").normalize().toString());
System.out.println("Solr home: " + SOLR_HOME.toString());
//The index is always stored within a temporary directory
@ -73,6 +84,13 @@ public abstract class AbstractEmbeddedSolrServerTestCase extends SolrTestCaseJ4
super.tearDown();
}
@AfterClass
public static void tearDownHome() throws Exception {
if (SOLR_HOME != null) {
FileUtils.deleteDirectory(SOLR_HOME.toFile());
}
}
protected void deleteAdditionalFiles() {
}

View File

@ -18,21 +18,29 @@ package org.apache.solr.client.solrj.request;
import java.io.File;
import java.lang.invoke.MethodHandles;
import java.util.Collection;
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
import com.codahale.metrics.MetricRegistry;
import org.apache.commons.io.FileUtils;
import org.apache.lucene.util.LuceneTestCase;
import org.apache.solr.SolrIgnoredThreadsFilter;
import org.apache.solr.client.solrj.SolrClient;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.embedded.AbstractEmbeddedSolrServerTestCase;
import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
import org.apache.solr.client.solrj.request.CoreAdminRequest.Create;
import org.apache.solr.client.solrj.response.CoreAdminResponse;
import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrDocumentList;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.core.SolrCore;
import org.apache.solr.metrics.SolrCoreMetricManager;
import org.apache.solr.metrics.SolrMetricManager;
import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Rule;
@ -186,6 +194,93 @@ public class TestCoreAdmin extends AbstractEmbeddedSolrServerTestCase {
assertTrue(exceptionMessage.contains("must consist entirely of periods, underscores, hyphens, and alphanumerics"));
}
}
@Test
public void testValidCoreRename() throws Exception {
Collection<String> names = cores.getAllCoreNames();
assertFalse(names.toString(), names.contains("coreRenamed"));
assertTrue(names.toString(), names.contains("core1"));
CoreAdminRequest.renameCore("core1", "coreRenamed", getSolrAdmin());
names = cores.getAllCoreNames();
assertTrue(names.toString(), names.contains("coreRenamed"));
assertFalse(names.toString(), names.contains("core1"));
// rename it back
CoreAdminRequest.renameCore("coreRenamed", "core1", getSolrAdmin());
names = cores.getAllCoreNames();
assertFalse(names.toString(), names.contains("coreRenamed"));
assertTrue(names.toString(), names.contains("core1"));
}
@Test
public void testCoreSwap() throws Exception {
// index marker docs to core0
SolrClient cli0 = getSolrCore0();
SolrInputDocument d = new SolrInputDocument("id", "core0-0");
cli0.add(d);
d = new SolrInputDocument("id", "core0-1");
cli0.add(d);
cli0.commit();
// index a marker doc to core1
SolrClient cli1 = getSolrCore1();
d = new SolrInputDocument("id", "core1-0");
cli1.add(d);
cli1.commit();
// initial state assertions
SolrQuery q = new SolrQuery("*:*");
QueryResponse rsp = cli0.query(q);
SolrDocumentList docs = rsp.getResults();
assertEquals(2, docs.size());
docs.forEach(doc -> {
assertTrue(doc.toString(), doc.getFieldValue("id").toString().startsWith("core0-"));
});
rsp = cli1.query(q);
docs = rsp.getResults();
assertEquals(1, docs.size());
docs.forEach(doc -> {
assertTrue(doc.toString(), doc.getFieldValue("id").toString().startsWith("core1-"));
});
// assert initial metrics
SolrMetricManager metricManager = cores.getMetricManager();
String core0RegistryName = SolrCoreMetricManager.createRegistryName(null, "core0");
String core1RegistryName = SolrCoreMetricManager.createRegistryName(null, "core1");
MetricRegistry core0Registry = metricManager.registry(core0RegistryName);
MetricRegistry core1Registry = metricManager.registry(core1RegistryName);
// 2 docs + 1 commit
assertEquals(3, core0Registry.counter("UPDATE./update.requests").getCount());
// 1 doc + 1 commit
assertEquals(2, core1Registry.counter("UPDATE./update.requests").getCount());
// swap
CoreAdminRequest.swapCore("core0", "core1", getSolrAdmin());
// assert state after swap
cli0 = getSolrCore0();
cli1 = getSolrCore1();
rsp = cli0.query(q);
docs = rsp.getResults();
assertEquals(1, docs.size());
docs.forEach(doc -> {
assertTrue(doc.toString(), doc.getFieldValue("id").toString().startsWith("core1-"));
});
rsp = cli1.query(q);
docs = rsp.getResults();
assertEquals(2, docs.size());
docs.forEach(doc -> {
assertTrue(doc.toString(), doc.getFieldValue("id").toString().startsWith("core0-"));
});
core0Registry = metricManager.registry(core0RegistryName);
core1Registry = metricManager.registry(core1RegistryName);
assertEquals(2, core0Registry.counter("UPDATE./update.requests").getCount());
assertEquals(3, core1Registry.counter("UPDATE./update.requests").getCount());
}
@BeforeClass
public static void before() {