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: [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]
