You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "mlbiscoc (via GitHub)" <gi...@apache.org> on 2025/11/12 20:59:40 UTC
[PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
mlbiscoc opened a new pull request, #3859:
URL: https://github.com/apache/solr/pull/3859
https://issues.apache.org/jira/browse/SOLR-17955
Splitshard disk space check was broken when switching the /admin/metrics api to prometheus. This fixes that by reading the parsed prometheus output.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc merged PR #3859:
URL: https://github.com/apache/solr/pull/3859
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3859:
URL: https://github.com/apache/solr/pull/3859#discussion_r2603468214
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -877,7 +879,11 @@ public static void checkDiskSpace(
SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
req.setResponseParser(new InputStreamResponseParser("prometheus"));
- SolrResponse resp = req.process(cloudManager.getSolrClient());
+ var cloudClient = (CloudHttp2SolrClient) cloudManager.getSolrClient();
+ var http2Client = (Http2SolrClient) cloudClient.getHttpClient();
Review Comment:
after you sync from main, please rename this var without the `2`. IMO we shouldn't have this number 2 in our client var names
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3859:
URL: https://github.com/apache/solr/pull/3859#discussion_r2629239930
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -880,12 +880,12 @@ public static void checkDiskSpace(
req.setResponseParser(new InputStreamResponseParser("prometheus"));
var cloudClient = (CloudHttp2SolrClient) cloudManager.getSolrClient();
- var http2Client = (Http2SolrClient) cloudClient.getHttpClient();
+ var httpClient = (HttpJettySolrClient) cloudClient.getHttpClient();
- SolrResponse resp =
- http2Client.requestWithBaseUrl(parentShardLeader.getBaseUrl(), req::process);
+ SolrResponse resp = httpClient.requestWithBaseUrl(parentShardLeader.getBaseUrl(), req::process);
Review Comment:
I wouldn't be surprised if we deprecate the method you are calling. It's overloaded; the other one is defined on the base client, is used somewhat widely, and returns a NamedList. Looking at the code here, you only want a NamedList, not SolrResponse specifically.
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -880,12 +880,12 @@ public static void checkDiskSpace(
req.setResponseParser(new InputStreamResponseParser("prometheus"));
var cloudClient = (CloudHttp2SolrClient) cloudManager.getSolrClient();
- var http2Client = (Http2SolrClient) cloudClient.getHttpClient();
+ var httpClient = (HttpJettySolrClient) cloudClient.getHttpClient();
Review Comment:
I suspect you no longer even need to cast... but whatever.
##########
changelog/unreleased/SOLR-17955-SplitShardCmd.checkDiskSpace fix.yml:
##########
@@ -0,0 +1,9 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: OTEL metrics - SplitShardCmd.checkDiskSpace needs conversion
+type: fixed
+authors:
+ - name: Matthew Biscocho
+ - name: David Smiley
+links:
+ - name: SOLR-17955
Review Comment:
maybe reference both JIRA issues that this addresses
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on code in PR #3859:
URL: https://github.com/apache/solr/pull/3859#discussion_r2728558274
##########
changelog/unreleased/SOLR-17955-SplitShardCmd.checkDiskSpace-otel.yml:
##########
@@ -0,0 +1,9 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: OTEL metrics - SplitShardCmd.checkDiskSpace needs conversion
+type: fixed
+authors:
+ - name: Matthew Biscocho
+ - name: David Smiley
+links:
+ - name: SOLR-17955
Review Comment:
I created another changelog entry instead
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on PR #3859:
URL: https://github.com/apache/solr/pull/3859#issuecomment-3666845343
Made last few changes on [46149ca](https://github.com/apache/solr/pull/3859/commits/46149ca9c53ba35cd8a7d779415952dcead1e54e) that I forgot to do.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3859:
URL: https://github.com/apache/solr/pull/3859#discussion_r2603457955
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -853,45 +857,61 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws Exception {
- if (true) {
- log.warn("checkDiskSpace disabled SOLR-17458 SOLR-17955");
- return;
- }
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- String replicaName = Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
- String indexSizeMetricName =
- "solr.core." + collection + "." + shard + "." + replicaName + ":INDEX.sizeInBytes";
- String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+ String indexSizeMetric = "solr_core_index_size_megabytes";
+ String freeDiskSpaceMetric = "solr_disk_space_megabytes";
+ String coreLabel =
+ collection
+ + "_"
+ + shard
+ + "_"
+ + Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
ModifiableSolrParams params =
- new ModifiableSolrParams()
- .add("key", indexSizeMetricName)
- .add("key", freeDiskSpaceMetricName);
- SolrResponse rsp =
+ new ModifiableSolrParams().add("name", indexSizeMetric).add("name", freeDiskSpaceMetric);
+
+ var req =
new GenericSolrRequest(
- SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params)
- .process(cloudManager.getSolrClient());
+ SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ SolrResponse resp = req.process(cloudManager.getSolrClient());
+
+ double[] sizes = new double[] {-1.0, -1.0}; // [indexSize, freeSize]
Review Comment:
it is too messy. In this case, use two AtomicDoubles
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3859:
URL: https://github.com/apache/solr/pull/3859#discussion_r2525736243
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -853,45 +857,61 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws Exception {
- if (true) {
- log.warn("checkDiskSpace disabled SOLR-17458 SOLR-17955");
- return;
- }
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- String replicaName = Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
- String indexSizeMetricName =
- "solr.core." + collection + "." + shard + "." + replicaName + ":INDEX.sizeInBytes";
- String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+ String indexSizeMetric = "solr_core_index_size_megabytes";
+ String freeDiskSpaceMetric = "solr_disk_space_megabytes";
+ String coreLabel =
+ collection
+ + "_"
+ + shard
+ + "_"
+ + Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
ModifiableSolrParams params =
- new ModifiableSolrParams()
- .add("key", indexSizeMetricName)
- .add("key", freeDiskSpaceMetricName);
- SolrResponse rsp =
+ new ModifiableSolrParams().add("name", indexSizeMetric).add("name", freeDiskSpaceMetric);
+
+ var req =
new GenericSolrRequest(
- SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params)
- .process(cloudManager.getSolrClient());
+ SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ SolrResponse resp = req.process(cloudManager.getSolrClient());
Review Comment:
this is going to talk to some arbitrary node, not necessarily the node that has the specific core we are going to split. To do that, you can get the URL from the Replica, truncate to the node, and use `Http2SolrClient.requestWithBaseUrl`
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -853,45 +857,61 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws Exception {
- if (true) {
- log.warn("checkDiskSpace disabled SOLR-17458 SOLR-17955");
- return;
- }
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- String replicaName = Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
- String indexSizeMetricName =
- "solr.core." + collection + "." + shard + "." + replicaName + ":INDEX.sizeInBytes";
- String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+ String indexSizeMetric = "solr_core_index_size_megabytes";
+ String freeDiskSpaceMetric = "solr_disk_space_megabytes";
+ String coreLabel =
+ collection
+ + "_"
+ + shard
+ + "_"
+ + Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
ModifiableSolrParams params =
- new ModifiableSolrParams()
- .add("key", indexSizeMetricName)
- .add("key", freeDiskSpaceMetricName);
- SolrResponse rsp =
+ new ModifiableSolrParams().add("name", indexSizeMetric).add("name", freeDiskSpaceMetric);
+
+ var req =
new GenericSolrRequest(
- SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params)
- .process(cloudManager.getSolrClient());
+ SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ SolrResponse resp = req.process(cloudManager.getSolrClient());
+
+ double[] sizes = new double[] {-1.0, -1.0}; // [indexSize, freeSize]
+ try (InputStream prometheusStream = (InputStream) resp.getResponse().get(STREAM_KEY);
+ var lines = NodeValueFetcher.Metrics.prometheusMetricStream(prometheusStream)) {
+
+ lines
+ .filter(line -> !line.isBlank() && !line.startsWith("#"))
+ .forEach(
+ line -> {
+ if (line.contains(indexSizeMetric) && line.contains(coreLabel)) {
+ sizes[0] = NodeValueFetcher.Metrics.extractPrometheusValue(line);
+ } else if (line.contains(freeDiskSpaceMetric) && line.contains("usable_space")) {
+ sizes[1] = NodeValueFetcher.Metrics.extractPrometheusValue(line);
+ }
+ });
+ }
+
+ double indexSize = sizes[0];
+ double freeSize = sizes[1];
- Number size = (Number) rsp.getResponse()._get(List.of("metrics", indexSizeMetricName), null);
- if (size == null) {
+ if (indexSize == -1.0) {
log.warn("cannot verify information for parent shard leader");
return;
}
- double indexSize = size.doubleValue();
- Number freeSize =
- (Number) rsp.getResponse()._get(List.of("metrics", freeDiskSpaceMetricName), null);
- if (freeSize == null) {
+ if (freeSize == -1.0) {
log.warn("missing node disk space information for parent shard leader");
return;
}
Review Comment:
At least `assert` but I'm +1 to just throw if you wish.
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -853,45 +857,61 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws Exception {
- if (true) {
- log.warn("checkDiskSpace disabled SOLR-17458 SOLR-17955");
- return;
- }
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- String replicaName = Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
- String indexSizeMetricName =
- "solr.core." + collection + "." + shard + "." + replicaName + ":INDEX.sizeInBytes";
- String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+ String indexSizeMetric = "solr_core_index_size_megabytes";
+ String freeDiskSpaceMetric = "solr_disk_space_megabytes";
+ String coreLabel =
+ collection
+ + "_"
+ + shard
+ + "_"
+ + Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
ModifiableSolrParams params =
- new ModifiableSolrParams()
- .add("key", indexSizeMetricName)
- .add("key", freeDiskSpaceMetricName);
- SolrResponse rsp =
+ new ModifiableSolrParams().add("name", indexSizeMetric).add("name", freeDiskSpaceMetric);
+
+ var req =
new GenericSolrRequest(
- SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params)
- .process(cloudManager.getSolrClient());
+ SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ SolrResponse resp = req.process(cloudManager.getSolrClient());
+
+ double[] sizes = new double[] {-1.0, -1.0}; // [indexSize, freeSize]
Review Comment:
why an array?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on code in PR #3859:
URL: https://github.com/apache/solr/pull/3859#discussion_r2590967805
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -853,45 +857,61 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws Exception {
- if (true) {
- log.warn("checkDiskSpace disabled SOLR-17458 SOLR-17955");
- return;
- }
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- String replicaName = Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
- String indexSizeMetricName =
- "solr.core." + collection + "." + shard + "." + replicaName + ":INDEX.sizeInBytes";
- String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+ String indexSizeMetric = "solr_core_index_size_megabytes";
+ String freeDiskSpaceMetric = "solr_disk_space_megabytes";
+ String coreLabel =
+ collection
+ + "_"
+ + shard
+ + "_"
+ + Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
ModifiableSolrParams params =
- new ModifiableSolrParams()
- .add("key", indexSizeMetricName)
- .add("key", freeDiskSpaceMetricName);
- SolrResponse rsp =
+ new ModifiableSolrParams().add("name", indexSizeMetric).add("name", freeDiskSpaceMetric);
+
+ var req =
new GenericSolrRequest(
- SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params)
- .process(cloudManager.getSolrClient());
+ SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ SolrResponse resp = req.process(cloudManager.getSolrClient());
+
+ double[] sizes = new double[] {-1.0, -1.0}; // [indexSize, freeSize]
Review Comment:
It was kind of a work around solution for the lambda filter. I can move off of it if you think it's too messy.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #3859:
URL: https://github.com/apache/solr/pull/3859#issuecomment-3793745796
@mlbiscoc this is waiting for > a month; missed 10.0
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "mlbiscoc (via GitHub)" <gi...@apache.org>.
mlbiscoc commented on code in PR #3859:
URL: https://github.com/apache/solr/pull/3859#discussion_r2519811150
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -853,45 +857,61 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws Exception {
- if (true) {
- log.warn("checkDiskSpace disabled SOLR-17458 SOLR-17955");
- return;
- }
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- String replicaName = Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
- String indexSizeMetricName =
- "solr.core." + collection + "." + shard + "." + replicaName + ":INDEX.sizeInBytes";
- String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+ String indexSizeMetric = "solr_core_index_size_megabytes";
+ String freeDiskSpaceMetric = "solr_disk_space_megabytes";
+ String coreLabel =
+ collection
+ + "_"
+ + shard
+ + "_"
+ + Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
ModifiableSolrParams params =
- new ModifiableSolrParams()
- .add("key", indexSizeMetricName)
- .add("key", freeDiskSpaceMetricName);
- SolrResponse rsp =
+ new ModifiableSolrParams().add("name", indexSizeMetric).add("name", freeDiskSpaceMetric);
+
+ var req =
new GenericSolrRequest(
- SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params)
- .process(cloudManager.getSolrClient());
+ SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ SolrResponse resp = req.process(cloudManager.getSolrClient());
+
+ double[] sizes = new double[] {-1.0, -1.0}; // [indexSize, freeSize]
+ try (InputStream prometheusStream = (InputStream) resp.getResponse().get(STREAM_KEY);
+ var lines = NodeValueFetcher.Metrics.prometheusMetricStream(prometheusStream)) {
+
+ lines
+ .filter(line -> !line.isBlank() && !line.startsWith("#"))
+ .forEach(
+ line -> {
+ if (line.contains(indexSizeMetric) && line.contains(coreLabel)) {
+ sizes[0] = NodeValueFetcher.Metrics.extractPrometheusValue(line);
+ } else if (line.contains(freeDiskSpaceMetric) && line.contains("usable_space")) {
+ sizes[1] = NodeValueFetcher.Metrics.extractPrometheusValue(line);
+ }
+ });
+ }
+
+ double indexSize = sizes[0];
+ double freeSize = sizes[1];
- Number size = (Number) rsp.getResponse()._get(List.of("metrics", indexSizeMetricName), null);
- if (size == null) {
+ if (indexSize == -1.0) {
log.warn("cannot verify information for parent shard leader");
return;
}
- double indexSize = size.doubleValue();
- Number freeSize =
- (Number) rsp.getResponse()._get(List.of("metrics", freeDiskSpaceMetricName), null);
- if (freeSize == null) {
+ if (freeSize == -1.0) {
log.warn("missing node disk space information for parent shard leader");
return;
}
Review Comment:
I'm thinking for here, this should throw instead of just return and continuing with the shard split cmd. Tests were passing because of this. Then we could at least know there was a regression. Was this just return for some specific purpose?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17955: OTEL metrics: SplitShardCmd.checkDiskSpace needs conversion [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3859:
URL: https://github.com/apache/solr/pull/3859#discussion_r2605026588
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -853,45 +857,61 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws Exception {
- if (true) {
- log.warn("checkDiskSpace disabled SOLR-17458 SOLR-17955");
- return;
- }
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- String replicaName = Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
- String indexSizeMetricName =
- "solr.core." + collection + "." + shard + "." + replicaName + ":INDEX.sizeInBytes";
- String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+ String indexSizeMetric = "solr_core_index_size_megabytes";
+ String freeDiskSpaceMetric = "solr_disk_space_megabytes";
+ String coreLabel =
+ collection
+ + "_"
+ + shard
+ + "_"
+ + Utils.parseMetricsReplicaName(collection, parentShardLeader.getCoreName());
ModifiableSolrParams params =
- new ModifiableSolrParams()
- .add("key", indexSizeMetricName)
- .add("key", freeDiskSpaceMetricName);
- SolrResponse rsp =
+ new ModifiableSolrParams().add("name", indexSizeMetric).add("name", freeDiskSpaceMetric);
+
+ var req =
new GenericSolrRequest(
- SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params)
- .process(cloudManager.getSolrClient());
+ SolrRequest.METHOD.GET, "/admin/metrics", SolrRequest.SolrRequestType.ADMIN, params);
+ req.setResponseParser(new InputStreamResponseParser("prometheus"));
+
+ SolrResponse resp = req.process(cloudManager.getSolrClient());
+
+ double[] sizes = new double[] {-1.0, -1.0}; // [indexSize, freeSize]
+ try (InputStream prometheusStream = (InputStream) resp.getResponse().get(STREAM_KEY);
+ var lines = NodeValueFetcher.Metrics.prometheusMetricStream(prometheusStream)) {
+
+ lines
+ .filter(line -> !line.isBlank() && !line.startsWith("#"))
+ .forEach(
+ line -> {
+ if (line.contains(indexSizeMetric) && line.contains(coreLabel)) {
+ sizes[0] = NodeValueFetcher.Metrics.extractPrometheusValue(line);
+ } else if (line.contains(freeDiskSpaceMetric) && line.contains("usable_space")) {
+ sizes[1] = NodeValueFetcher.Metrics.extractPrometheusValue(line);
+ }
+ });
+ }
+
+ double indexSize = sizes[0];
+ double freeSize = sizes[1];
- Number size = (Number) rsp.getResponse()._get(List.of("metrics", indexSizeMetricName), null);
- if (size == null) {
+ if (indexSize == -1.0) {
log.warn("cannot verify information for parent shard leader");
return;
}
- double indexSize = size.doubleValue();
- Number freeSize =
- (Number) rsp.getResponse()._get(List.of("metrics", freeDiskSpaceMetricName), null);
- if (freeSize == null) {
+ if (freeSize == -1.0) {
log.warn("missing node disk space information for parent shard leader");
return;
}
Review Comment:
As a general rule, never log AND throw. Pick one. Here, you choose to throw, so ditch the log. Logging is typically redundant with throwing, since throwing basically always gets logged as well.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org