dsmiley commented on code in PR #1485:
URL: https://github.com/apache/solr/pull/1485#discussion_r1146665022
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -873,42 +873,52 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws SolrException {
+
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- NodeStateProvider nodeStateProvider = cloudManager.getNodeStateProvider();
- Map<String, Object> nodeValues =
- nodeStateProvider.getNodeValues(
- parentShardLeader.getNodeName(),
Collections.singletonList(ImplicitSnitch.DISK));
- Map<String, Map<String, List<Replica>>> infos =
- nodeStateProvider.getReplicaInfo(
- parentShardLeader.getNodeName(),
Collections.singletonList(CORE_IDX.metricsAttribute));
- if (infos.get(collection) == null || infos.get(collection).get(shard) ==
null) {
- log.warn("cannot verify information for parent shard leader");
+
+ String indexSizeMetricName =
+ "solr.core."
+ + collection
+ + "."
+ + shard
+ + "."
+ + Utils.parseMetricsReplicaName(collection,
parentShardLeader.getCoreName())
+ + ":INDEX.sizeInBytes";
+ String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+
+ ModifiableSolrParams params =
+ new ModifiableSolrParams()
+ .add("key", indexSizeMetricName)
+ .add("key", freeDiskSpaceMetricName);
+ SolrResponse rsp;
+ try {
+ rsp =
+ cloudManager.request(
+ new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/metrics",
params));
+ } catch (Exception e) {
+ log.error("Error occurred while checking the disk space of the node");
return;
}
- // find the leader
- List<Replica> lst = infos.get(collection).get(shard);
- Double indexSize = null;
- for (Replica info : lst) {
- if (info.getCoreName().equals(parentShardLeader.getCoreName())) {
- Number size = (Number) info.get(CORE_IDX.metricsAttribute);
- if (size == null) {
- log.warn("cannot verify information for parent shard leader");
- return;
- }
- indexSize = (Double) CORE_IDX.convertVal(size);
- break;
- }
+ if (rsp.getResponse() == null) {
+ log.warn("cannot verify information for parent shard leader");
+ return;
}
Review Comment:
I would love to get rid of this. There will be a response. If it can be
shown that there isn't in weird cases, I'd rather improve such weird cases so
Solr code can make such guarantees without needless/verbose null checks.
You could not declare the "rsp" even; just immediately grab the
all-important NamedList.
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -873,42 +873,52 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws SolrException {
+
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- NodeStateProvider nodeStateProvider = cloudManager.getNodeStateProvider();
- Map<String, Object> nodeValues =
- nodeStateProvider.getNodeValues(
- parentShardLeader.getNodeName(),
Collections.singletonList(ImplicitSnitch.DISK));
- Map<String, Map<String, List<Replica>>> infos =
- nodeStateProvider.getReplicaInfo(
- parentShardLeader.getNodeName(),
Collections.singletonList(CORE_IDX.metricsAttribute));
- if (infos.get(collection) == null || infos.get(collection).get(shard) ==
null) {
- log.warn("cannot verify information for parent shard leader");
+
+ String indexSizeMetricName =
+ "solr.core."
+ + collection
+ + "."
+ + shard
+ + "."
+ + Utils.parseMetricsReplicaName(collection,
parentShardLeader.getCoreName())
Review Comment:
declare this as its own variable so that there isn't this silly long line
breaking. Or String.format if you prefer would still be a substantial
improvement.
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java:
##########
@@ -873,42 +873,52 @@ public static void checkDiskSpace(
SolrIndexSplitter.SplitMethod method,
SolrCloudManager cloudManager)
throws SolrException {
+
// check that enough disk space is available on the parent leader node
// otherwise the actual index splitting will always fail
- NodeStateProvider nodeStateProvider = cloudManager.getNodeStateProvider();
- Map<String, Object> nodeValues =
- nodeStateProvider.getNodeValues(
- parentShardLeader.getNodeName(),
Collections.singletonList(ImplicitSnitch.DISK));
- Map<String, Map<String, List<Replica>>> infos =
- nodeStateProvider.getReplicaInfo(
- parentShardLeader.getNodeName(),
Collections.singletonList(CORE_IDX.metricsAttribute));
- if (infos.get(collection) == null || infos.get(collection).get(shard) ==
null) {
- log.warn("cannot verify information for parent shard leader");
+
+ String indexSizeMetricName =
+ "solr.core."
+ + collection
+ + "."
+ + shard
+ + "."
+ + Utils.parseMetricsReplicaName(collection,
parentShardLeader.getCoreName())
+ + ":INDEX.sizeInBytes";
+ String freeDiskSpaceMetricName = "solr.node:CONTAINER.fs.usableSpace";
+
+ ModifiableSolrParams params =
+ new ModifiableSolrParams()
+ .add("key", indexSizeMetricName)
+ .add("key", freeDiskSpaceMetricName);
+ SolrResponse rsp;
+ try {
+ rsp =
+ cloudManager.request(
+ new GenericSolrRequest(SolrRequest.METHOD.GET, "/admin/metrics",
params));
+ } catch (Exception e) {
+ log.error("Error occurred while checking the disk space of the node");
Review Comment:
always propagate the exception!
--
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...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
