Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
mlbiscoc merged PR #3458: URL: https://github.com/apache/solr/pull/3458 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
mlbiscoc commented on PR #3458: URL: https://github.com/apache/solr/pull/3458#issuecomment-3225347084 Anything else here? I would like to merge this because it makes some changes to Solr `SolrMetricsIntegrationTest` that was a bad apple and helps unblock `CoreContainer` [metrics migration and its tests](https://github.com/apache/solr/pull/3509). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
dsmiley commented on code in PR #3458:
URL: https://github.com/apache/solr/pull/3458#discussion_r2280729254
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -105,30 +112,35 @@ public void loadReporters() {
}
/**
- * Make sure that metrics already collected that correspond to the old core
name are carried over
- * and will be used under the new core name. This method also reloads
reporters so that they use
- * the new core name.
+ * Re-register all metric producers associated with this core. This
recreates the metric registry
+ * resetting its state
*/
- // NOCOMMIT SOLR-17458: Update for core renaming
- public void afterCoreRename() {
-assert core.getCoreDescriptor().getCloudDescriptor() == null;
-String oldRegistryName = solrMetricsContext.getRegistryName();
-String oldLeaderRegistryName = leaderRegistryName;
-String newRegistryName =
-createRegistryName(cloudMode, collectionName, shardName, replicaName,
core.getName());
-leaderRegistryName = createLeaderRegistryName(cloudMode, collectionName,
shardName);
-if (oldRegistryName.equals(newRegistryName)) {
- return;
-}
-// close old reporters
-metricManager.closeReporters(oldRegistryName, solrMetricsContext.getTag());
-if (oldLeaderRegistryName != null) {
- metricManager.closeReporters(oldLeaderRegistryName,
solrMetricsContext.getTag());
-}
-solrMetricsContext =
-new SolrMetricsContext(metricManager, newRegistryName,
solrMetricsContext.getTag());
-// load reporters again, using the new core name
-loadReporters();
+ public void reregisterCoreMetrics() {
+this.solrMetricsContext =
+new SolrMetricsContext(
+metricManager,
+createRegistryName(cloudMode, collectionName, shardName,
replicaName, core.getName()),
+solrMetricsContext.getTag());
+metricManager.removeRegistry(solrMetricsContext.getRegistryName());
+if (leaderRegistryName != null)
metricManager.removeRegistry(leaderRegistryName);
+
+var attributesBuilder =
+Attributes.builder()
+.put(CORE_ATTR, core.getName())
Review Comment:
my point about the duplication, for example, can be seen in the attributes
construction. This is duplicated over in `registerMetricProcducer`. But it's
not a lot of duplication. It could be ameliorated with comments on both sides.
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -105,30 +112,35 @@ public void loadReporters() {
}
/**
- * Make sure that metrics already collected that correspond to the old core
name are carried over
- * and will be used under the new core name. This method also reloads
reporters so that they use
- * the new core name.
+ * Re-register all metric producers associated with this core. This
recreates the metric registry
+ * resetting its state
*/
- // NOCOMMIT SOLR-17458: Update for core renaming
- public void afterCoreRename() {
-assert core.getCoreDescriptor().getCloudDescriptor() == null;
-String oldRegistryName = solrMetricsContext.getRegistryName();
-String oldLeaderRegistryName = leaderRegistryName;
-String newRegistryName =
-createRegistryName(cloudMode, collectionName, shardName, replicaName,
core.getName());
-leaderRegistryName = createLeaderRegistryName(cloudMode, collectionName,
shardName);
-if (oldRegistryName.equals(newRegistryName)) {
- return;
-}
-// close old reporters
-metricManager.closeReporters(oldRegistryName, solrMetricsContext.getTag());
-if (oldLeaderRegistryName != null) {
- metricManager.closeReporters(oldLeaderRegistryName,
solrMetricsContext.getTag());
-}
-solrMetricsContext =
-new SolrMetricsContext(metricManager, newRegistryName,
solrMetricsContext.getTag());
-// load reporters again, using the new core name
-loadReporters();
+ public void reregisterCoreMetrics() {
+this.solrMetricsContext =
+new SolrMetricsContext(
+metricManager,
+createRegistryName(cloudMode, collectionName, shardName,
replicaName, core.getName()),
+solrMetricsContext.getTag());
+metricManager.removeRegistry(solrMetricsContext.getRegistryName());
+if (leaderRegistryName != null)
metricManager.removeRegistry(leaderRegistryName);
+
+var attributesBuilder =
Review Comment:
you should just build this; have it be the attributes itself. You're
building in a loop below but it's redundant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
dsmiley commented on code in PR #3458:
URL: https://github.com/apache/solr/pull/3458#discussion_r2279139483
##
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java:
##
@@ -74,6 +74,9 @@ public Map getSysProps(String nodeName,
Collection tags)
return result;
}
+ // NOCOMMIT: These properties were fetched from the /admin/metrics endpoint.
These properties were
+ // stored as strings instead of numeric values. This is not possible in OTEL
metrics. Need to
+ // revisit this later.
private Map fetchProps(String nodeName, Collection
tags) {
Review Comment:
Please insert this recommendation into the TODO comment, then we move on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
dsmiley commented on code in PR #3458:
URL: https://github.com/apache/solr/pull/3458#discussion_r2279136818
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -52,6 +53,11 @@ public class SolrCoreMetricManager implements Closeable {
private String leaderRegistryName;
private boolean cloudMode;
+ // Track all metric producers registered for this core so we can
re-initialize them during rename
+ private final List registeredProducers = new
ArrayList<>();
Review Comment:
(pending)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
mlbiscoc commented on code in PR #3458:
URL: https://github.com/apache/solr/pull/3458#discussion_r2263737412
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -52,6 +53,11 @@ public class SolrCoreMetricManager implements Closeable {
private String leaderRegistryName;
private boolean cloudMode;
+ // Track all metric producers registered for this core so we can
re-initialize them during rename
+ private final List registeredProducers = new
ArrayList<>();
Review Comment:
In Dropwizard, the "core registries" only needed to be renamed on the
registries itself which worked because nothing had labels. Not possible in
OTEL. We need to track our metric producers so we can re-register and
reinitialize its metrics with the correct core name for labels.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
mlbiscoc commented on PR #3458: URL: https://github.com/apache/solr/pull/3458#issuecomment-3175206557 > there were nasty/insiduous memory leaks relating to metrics for cores that closed and/or core swapping Will make a Jira to double check this. I'll take a look at the previous jira that found this and what happened. Maybe will do some JFR testing to confirm and investigate. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
mlbiscoc commented on code in PR #3458:
URL: https://github.com/apache/solr/pull/3458#discussion_r2266979434
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -105,30 +111,35 @@ public void loadReporters() {
}
/**
- * Make sure that metrics already collected that correspond to the old core
name are carried over
- * and will be used under the new core name. This method also reloads
reporters so that they use
- * the new core name.
+ * Re-register all metric producers associated with this core. This
recreates the metric registry
+ * resetting its state
*/
- // NOCOMMIT SOLR-17458: Update for core renaming
- public void afterCoreRename() {
-assert core.getCoreDescriptor().getCloudDescriptor() == null;
-String oldRegistryName = solrMetricsContext.getRegistryName();
-String oldLeaderRegistryName = leaderRegistryName;
-String newRegistryName =
-createRegistryName(cloudMode, collectionName, shardName, replicaName,
core.getName());
-leaderRegistryName = createLeaderRegistryName(cloudMode, collectionName,
shardName);
-if (oldRegistryName.equals(newRegistryName)) {
- return;
-}
-// close old reporters
-metricManager.closeReporters(oldRegistryName, solrMetricsContext.getTag());
-if (oldLeaderRegistryName != null) {
- metricManager.closeReporters(oldLeaderRegistryName,
solrMetricsContext.getTag());
-}
-solrMetricsContext =
-new SolrMetricsContext(metricManager, newRegistryName,
solrMetricsContext.getTag());
-// load reporters again, using the new core name
-loadReporters();
+ public void reregisterCoreMetrics() {
Review Comment:
We can't remove metrics from an existing registry after creation so we have
to just completely tear it down and recreate it.
##
solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java:
##
@@ -68,6 +69,42 @@ public static Map getRandomMetrics(Random
random, boolean shoul
return shouldDefineMetrics ? getRandomMetricsWithReplacements(random, new
HashMap<>()) : null;
}
+ /**
+ * Generate random OpenTelemetry metric names for testing Prometheus
metrics. Returns a map of
+ * metric names to their expected increment values.
+ */
+ public static Map getRandomPrometheusMetrics(Random random) {
+return getRandomPrometheusMetrics(random, random.nextBoolean());
+ }
+
+ public static Map getRandomPrometheusMetrics(
+ Random random, boolean shouldDefineMetrics) {
+return shouldDefineMetrics
+? getRandomPrometheusMetricsWithReplacements(random, new HashMap<>())
+: null;
+ }
+
+ public static Map getRandomPrometheusMetricsWithReplacements(
+ Random random, Map existing) {
+HashMap metrics = new HashMap<>();
+ArrayList existingKeys = new ArrayList<>(existing.keySet());
+
+int numMetrics = TestUtil.nextInt(random, 1, MAX_ITERATIONS);
+for (int i = 0; i < numMetrics; ++i) {
+ boolean shouldReplaceMetric = !existing.isEmpty() &&
random.nextBoolean();
+ String name =
+ shouldReplaceMetric
+ ? existingKeys.get(TestUtil.nextInt(random, 0,
existingKeys.size() - 1))
Review Comment:
`!existing.isEmpty()` is checked right above so it shouldn't fail.
##
solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java:
##
@@ -68,6 +69,42 @@ public static Map getRandomMetrics(Random
random, boolean shoul
return shouldDefineMetrics ? getRandomMetricsWithReplacements(random, new
HashMap<>()) : null;
}
+ /**
+ * Generate random OpenTelemetry metric names for testing Prometheus
metrics. Returns a map of
+ * metric names to their expected increment values.
+ */
+ public static Map getRandomPrometheusMetrics(Random random) {
+return getRandomPrometheusMetrics(random, random.nextBoolean());
+ }
+
+ public static Map getRandomPrometheusMetrics(
+ Random random, boolean shouldDefineMetrics) {
+return shouldDefineMetrics
+? getRandomPrometheusMetricsWithReplacements(random, new HashMap<>())
+: null;
+ }
+
+ public static Map getRandomPrometheusMetricsWithReplacements(
+ Random random, Map existing) {
+HashMap metrics = new HashMap<>();
+ArrayList existingKeys = new ArrayList<>(existing.keySet());
+
+int numMetrics = TestUtil.nextInt(random, 1, MAX_ITERATIONS);
+for (int i = 0; i < numMetrics; ++i) {
+ boolean shouldReplaceMetric = !existing.isEmpty() &&
random.nextBoolean();
+ String name =
+ shouldReplaceMetric
+ ? existingKeys.get(TestUtil.nextInt(random, 0,
existingKeys.size() - 1))
+ : TestUtil.randomSimpleString(random, 5, 10)
+ + SUFFIX; // must be simple string for JMX publishing
+
+ Long incrementValue = Math.abs(random.nextLong() % 1000) + 1;
Review Comment:
Did you mean TestUtil.nextLong? Updated to that.
##
solr/core/src/test/org/apache/
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
dsmiley commented on code in PR #3458:
URL: https://github.com/apache/solr/pull/3458#discussion_r2264034281
##
solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java:
##
@@ -68,6 +69,42 @@ public static Map getRandomMetrics(Random
random, boolean shoul
return shouldDefineMetrics ? getRandomMetricsWithReplacements(random, new
HashMap<>()) : null;
}
+ /**
+ * Generate random OpenTelemetry metric names for testing Prometheus
metrics. Returns a map of
+ * metric names to their expected increment values.
+ */
+ public static Map getRandomPrometheusMetrics(Random random) {
+return getRandomPrometheusMetrics(random, random.nextBoolean());
+ }
+
+ public static Map getRandomPrometheusMetrics(
Review Comment:
this looks useless. Pass a false and it always return null. Who would
explicitly call it as such? If it only exists for the random case above then
inline it (which IntelliJ can do in a single action).
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -52,6 +53,11 @@ public class SolrCoreMetricManager implements Closeable {
private String leaderRegistryName;
private boolean cloudMode;
+ // Track all metric producers registered for this core so we can
re-initialize them during rename
+ private final List registeredProducers = new
ArrayList<>();
Review Comment:
I suggest improving this comment to say "**core** rename". "rename" alone
is ambiguous.
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -105,30 +111,35 @@ public void loadReporters() {
}
/**
- * Make sure that metrics already collected that correspond to the old core
name are carried over
- * and will be used under the new core name. This method also reloads
reporters so that they use
- * the new core name.
+ * Re-register all metric producers associated with this core. This
recreates the metric registry
+ * resetting its state
*/
- // NOCOMMIT SOLR-17458: Update for core renaming
- public void afterCoreRename() {
-assert core.getCoreDescriptor().getCloudDescriptor() == null;
-String oldRegistryName = solrMetricsContext.getRegistryName();
-String oldLeaderRegistryName = leaderRegistryName;
-String newRegistryName =
-createRegistryName(cloudMode, collectionName, shardName, replicaName,
core.getName());
-leaderRegistryName = createLeaderRegistryName(cloudMode, collectionName,
shardName);
-if (oldRegistryName.equals(newRegistryName)) {
- return;
-}
-// close old reporters
-metricManager.closeReporters(oldRegistryName, solrMetricsContext.getTag());
-if (oldLeaderRegistryName != null) {
- metricManager.closeReporters(oldLeaderRegistryName,
solrMetricsContext.getTag());
-}
-solrMetricsContext =
-new SolrMetricsContext(metricManager, newRegistryName,
solrMetricsContext.getTag());
-// load reporters again, using the new core name
-loadReporters();
+ public void reregisterCoreMetrics() {
Review Comment:
I would have hoped that removing then adding metrics allows us to use the
very same code we already have for those separate things.
##
solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java:
##
@@ -68,6 +69,42 @@ public static Map getRandomMetrics(Random
random, boolean shoul
return shouldDefineMetrics ? getRandomMetricsWithReplacements(random, new
HashMap<>()) : null;
}
+ /**
+ * Generate random OpenTelemetry metric names for testing Prometheus
metrics. Returns a map of
+ * metric names to their expected increment values.
+ */
+ public static Map getRandomPrometheusMetrics(Random random) {
+return getRandomPrometheusMetrics(random, random.nextBoolean());
+ }
+
+ public static Map getRandomPrometheusMetrics(
+ Random random, boolean shouldDefineMetrics) {
+return shouldDefineMetrics
+? getRandomPrometheusMetricsWithReplacements(random, new HashMap<>())
+: null;
+ }
+
+ public static Map getRandomPrometheusMetricsWithReplacements(
+ Random random, Map existing) {
+HashMap metrics = new HashMap<>();
+ArrayList existingKeys = new ArrayList<>(existing.keySet());
+
+int numMetrics = TestUtil.nextInt(random, 1, MAX_ITERATIONS);
+for (int i = 0; i < numMetrics; ++i) {
+ boolean shouldReplaceMetric = !existing.isEmpty() &&
random.nextBoolean();
+ String name =
+ shouldReplaceMetric
+ ? existingKeys.get(TestUtil.nextInt(random, 0,
existingKeys.size() - 1))
+ : TestUtil.randomSimpleString(random, 5, 10)
+ + SUFFIX; // must be simple string for JMX publishing
+
+ Long incrementValue = Math.abs(random.nextLong() % 1000) + 1;
Review Comment:
TextUtil.nextLong
##
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java:
##
@@ -74,6 +74,9 @@ public Map getSys
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
mlbiscoc commented on code in PR #3458: URL: https://github.com/apache/solr/pull/3458#discussion_r2263746534 ## solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java: ## Review Comment: This was a headache to migrate. Will need to come back to finish it later -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
mlbiscoc commented on code in PR #3458:
URL: https://github.com/apache/solr/pull/3458#discussion_r2263737412
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -52,6 +53,11 @@ public class SolrCoreMetricManager implements Closeable {
private String leaderRegistryName;
private boolean cloudMode;
+ // Track all metric producers registered for this core so we can
re-initialize them during rename
+ private final List registeredProducers = new
ArrayList<>();
Review Comment:
In Dropwizard, the registries only needed to be renamed on the registries
itself which worked because nothing had labels. Not possible in OTEL. We need
to track our metric producers so we can re-register and reinitialize its
metrics with the correct core name for labels.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17806: Recreate OTEL metric registries on core rename/swapping [solr]
mlbiscoc commented on code in PR #3458:
URL: https://github.com/apache/solr/pull/3458#discussion_r2263737412
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -52,6 +53,11 @@ public class SolrCoreMetricManager implements Closeable {
private String leaderRegistryName;
private boolean cloudMode;
+ // Track all metric producers registered for this core so we can
re-initialize them during rename
+ private final List registeredProducers = new
ArrayList<>();
Review Comment:
In Dropwizard, the registries only needed to be renamed on which worked
because nothing had labels. Not possible in OTEL. We need to track our metric
producers so we can re-register and reinitialize its metrics with the correct
core name for labels.
##
solr/core/src/java/org/apache/solr/metrics/SolrCoreMetricManager.java:
##
@@ -147,11 +158,12 @@ public void registerMetricProducer(String scope,
SolrMetricProducer producer) {
+ producer);
}
-// NOCOMMIT SOLR-17458: These attributes may not work for standalone mode
and maybe make the
Review Comment:
Turns out you try putting a `null` value for a label it just gets ignored so
this always worked in standalone mode.
##
solr/core/src/test/org/apache/solr/metrics/SolrMetricsIntegrationTest.java:
##
Review Comment:
This was a headache to migrate. Will need to come back to finish it later
##
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/NodesSysPropsCacher.java:
##
@@ -74,6 +74,9 @@ public Map getSysProps(String nodeName,
Collection tags)
return result;
}
+ // NOCOMMIT: These properties were fetched from the /admin/metrics endpoint.
These properties were
+ // stored as strings instead of numeric values. This is not possible in OTEL
metrics. Need to
+ // revisit this later.
private Map fetchProps(String nodeName, Collection
tags) {
Review Comment:
Not sure if these properties can be retrieved from somewhere else but it
cannot be from the /admin/metrics endpoint. Metrics are only numeric values
that can be aggregated so it didn't really make sense that the metrics had
strings as values. Is there a some endpoint available to get these system props
instead?
##
solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java:
##
@@ -755,68 +755,15 @@ private static MetricRegistry getOrCreateRegistry(
*
* @param registry name of the registry to remove
*/
- // TODO SOLR-17458: You can't delete OTEL meters
public void removeRegistry(String registry) {
-// NOCOMMIT Remove all closing Dropwizard registries
-// close any reporters for this registry first
-closeReporters(registry, null);
-// make sure we use a name with prefix
-registry = enforcePrefix(registry);
-if (isSharedRegistry(registry)) {
- SharedMetricRegistries.remove(registry);
-} else {
- swapLock.lock();
- try {
-registries.remove(registry);
- } finally {
-swapLock.unlock();
- }
-}
meterProviderAndReaders.computeIfPresent(
-registry,
+enforcePrefix(registry),
(key, meterAndReader) -> {
meterAndReader.sdkMeterProvider().close();
return null;
});
}
- /**
- * 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.
- */
- // NOCOMMIT SOLR-17458: Don't think we need
- public void swapRegistries(String registry1, String registry2) {
Review Comment:
No such thing as "swapping or renaming registries" just recreating now.
##
solr/core/src/test/org/apache/solr/metrics/SolrMetricTestUtils.java:
##
@@ -193,4 +230,86 @@ public static HistogramSnapshot.HistogramDataPointSnapshot
getHistogramDatapoint
return getDatapoint(
core, metricName, labels,
HistogramSnapshot.HistogramDataPointSnapshot.class);
}
+
+ public static CounterSnapshot.CounterDataPointSnapshot
getStandaloneSelectRequestsDatapoint(
Review Comment:
I found myself very often using `select` and `update` requests as easy ways
of asserting these registries were getting updated values. Just created these
util methods to help. Will probably go back at the other tests that checked
these and just call this instead of constantly recreating the same set of
labels.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@
