Browse Source

Rename the fields reported under details by the disk indicator (#90717)

Currently, we report the count of affected nodes and indices as part of
the disk indicator using a leaky abstraction. Namely we use the status
we assign to nodes internally to nodes based on their disk usage (red,
yellow, green, unknown).

However, these statuses don't have an explicit meaning outside the
implementation details e.g. a red node would probably convey it's a node
experiencing disk issues but not what kind

This proposes being explicit in what we return to our health API users
e.g.
```
"details": {
  "indices_with_readonly_block": 2,
  "nodes_with_enough_disk_space": 0,
  "nodes_with_unknown_disk_status": 0,
  "nodes_over_high_watermark": 0,
  "nodes_over_flood_watermark": 2
}
```
Andrei Dan 3 years ago
parent
commit
b55f5fd77b

+ 10 - 7
docs/reference/health/health.asciidoc

@@ -317,17 +317,20 @@ details have contents and a structure that is unique to each indicator.
 [[health-api-response-details-disk]]
 ===== disk
 
-`blocked_indices`::
-(int) The number of indices that are blocked by the system because the cluster is running out of space.
+`indices_with_readonly_block`::
+(int) The number of indices the system enforced a read-only index block (`index.blocks.read_only_allow_delete`) on
+because the cluster is running out of space.
 
-`green_nodes`::
+`nodes_with_enough_disk_space`::
 (int) The number of nodes that have enough available disk space to function.
 
-`yellow_nodes`::
-(int) The number of nodes that are running low on disk and it is likely that they will run out of space.
+`nodes_over_high_watermark`::
+(int) The number of nodes that are running low on disk and it is likely that they will run out of space. Their disk usage
+has tripped the <<cluster-routing-watermark-high, high watermark threshold>>.
 
-`red_nodes`::
-(int) The number of nodes that have run out of disk.
+`nodes_over_flood_stage_watermark`::
+(int) The number of nodes that have run out of disk. Their disk usage has tripped the <<cluster-routing-flood-stage, flood stage
+watermark threshold>>.
 
 `unknown_nodes`::
 (int) The number of nodes for which it was not possible to determine their disk health.

+ 17 - 2
server/src/main/java/org/elasticsearch/health/node/DiskHealthIndicatorService.java

@@ -134,6 +134,12 @@ public class DiskHealthIndicatorService implements HealthIndicatorService {
      */
     static class DiskHealthAnalyzer {
 
+        public static final String INDICES_WITH_READONLY_BLOCK = "indices_with_readonly_block";
+        public static final String NODES_WITH_ENOUGH_DISK_SPACE = "nodes_with_enough_disk_space";
+        public static final String NODES_OVER_FLOOD_STAGE_WATERMARK = "nodes_over_flood_stage_watermark";
+        public static final String NODES_OVER_HIGH_WATERMARK = "nodes_over_high_watermark";
+        public static final String NODES_WITH_UNKNOWN_DISK_STATUS = "nodes_with_unknown_disk_status";
+
         private final ClusterState clusterState;
         private final Set<String> blockedIndices;
         private final List<DiscoveryNode> dataNodes = new ArrayList<>();
@@ -395,14 +401,23 @@ public class DiskHealthIndicatorService implements HealthIndicatorService {
             }
             return ((builder, params) -> {
                 builder.startObject();
-                builder.field("blocked_indices", blockedIndices.size());
+                builder.field(INDICES_WITH_READONLY_BLOCK, blockedIndices.size());
                 for (HealthStatus healthStatus : HealthStatus.values()) {
-                    builder.field(healthStatus.name().toLowerCase(Locale.ROOT) + "_nodes", healthNodesCount.get(healthStatus));
+                    builder.field(getDetailsDisplayKey(healthStatus), healthNodesCount.get(healthStatus));
                 }
                 return builder.endObject();
             });
         }
 
+        private static String getDetailsDisplayKey(HealthStatus status) {
+            return switch (status) {
+                case GREEN -> NODES_WITH_ENOUGH_DISK_SPACE;
+                case UNKNOWN -> NODES_WITH_UNKNOWN_DISK_STATUS;
+                case YELLOW -> NODES_OVER_HIGH_WATERMARK;
+                case RED -> NODES_OVER_FLOOD_STAGE_WATERMARK;
+            };
+        }
+
         private boolean hasUnhealthyDataNodes() {
             return dataNodes.isEmpty() == false;
         }

+ 35 - 30
server/src/test/java/org/elasticsearch/health/node/DiskHealthIndicatorServiceTests.java

@@ -57,6 +57,11 @@ import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_CREATION_
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
 import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED;
+import static org.elasticsearch.health.node.DiskHealthIndicatorService.DiskHealthAnalyzer.INDICES_WITH_READONLY_BLOCK;
+import static org.elasticsearch.health.node.DiskHealthIndicatorService.DiskHealthAnalyzer.NODES_OVER_FLOOD_STAGE_WATERMARK;
+import static org.elasticsearch.health.node.DiskHealthIndicatorService.DiskHealthAnalyzer.NODES_OVER_HIGH_WATERMARK;
+import static org.elasticsearch.health.node.DiskHealthIndicatorService.DiskHealthAnalyzer.NODES_WITH_ENOUGH_DISK_SPACE;
+import static org.elasticsearch.health.node.DiskHealthIndicatorService.DiskHealthAnalyzer.NODES_WITH_UNKNOWN_DISK_STATUS;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -142,11 +147,11 @@ public class DiskHealthIndicatorServiceTests extends ESTestCase {
         assertThat(result.impacts().size(), equalTo(0));
         assertThat(result.diagnosisList().size(), equalTo(0));
         Map<String, Object> details = xContentToMap(result.details());
-        assertThat(details.get("green_nodes"), equalTo(discoveryNodes.size()));
-        assertThat(details.get("unknown_nodes"), equalTo(0));
-        assertThat(details.get("yellow_nodes"), equalTo(0));
-        assertThat(details.get("red_nodes"), equalTo(0));
-        assertThat(details.get("blocked_indices"), equalTo(0));
+        assertThat(details.get(NODES_WITH_ENOUGH_DISK_SPACE), equalTo(discoveryNodes.size()));
+        assertThat(details.get(NODES_WITH_UNKNOWN_DISK_STATUS), equalTo(0));
+        assertThat(details.get(NODES_OVER_HIGH_WATERMARK), equalTo(0));
+        assertThat(details.get(NODES_OVER_FLOOD_STAGE_WATERMARK), equalTo(0));
+        assertThat(details.get(INDICES_WITH_READONLY_BLOCK), equalTo(0));
     }
 
     /*
@@ -220,11 +225,11 @@ public class DiskHealthIndicatorServiceTests extends ESTestCase {
                 assertThat(affectedResources.get(0).getNodes(), equalTo(affectedNodes));
             }
             Map<String, Object> details = xContentToMap(result.details());
-            assertThat(details.get("green_nodes"), equalTo(0));
-            assertThat(details.get("unknown_nodes"), equalTo(0));
-            assertThat(details.get("yellow_nodes"), equalTo(allNodes.size()));
-            assertThat(details.get("red_nodes"), equalTo(0));
-            assertThat(details.get("blocked_indices"), equalTo(0));
+            assertThat(details.get(NODES_WITH_ENOUGH_DISK_SPACE), equalTo(0));
+            assertThat(details.get(NODES_WITH_UNKNOWN_DISK_STATUS), equalTo(0));
+            assertThat(details.get(NODES_OVER_HIGH_WATERMARK), equalTo(allNodes.size()));
+            assertThat(details.get(NODES_WITH_ENOUGH_DISK_SPACE), equalTo(0));
+            assertThat(details.get(INDICES_WITH_READONLY_BLOCK), equalTo(0));
         }
     }
 
@@ -294,11 +299,11 @@ public class DiskHealthIndicatorServiceTests extends ESTestCase {
         );
 
         Map<String, Object> details = xContentToMap(result.details());
-        assertThat(details.get("green_nodes"), equalTo(discoveryNodes.size() - affectedNodes.size()));
-        assertThat(details.get("unknown_nodes"), equalTo(0));
-        assertThat(details.get("yellow_nodes"), equalTo(0));
-        assertThat(details.get("red_nodes"), equalTo(affectedNodes.size()));
-        assertThat(details.get("blocked_indices"), equalTo(0));
+        assertThat(details.get(NODES_WITH_ENOUGH_DISK_SPACE), equalTo(discoveryNodes.size() - affectedNodes.size()));
+        assertThat(details.get(NODES_WITH_UNKNOWN_DISK_STATUS), equalTo(0));
+        assertThat(details.get(NODES_OVER_HIGH_WATERMARK), equalTo(0));
+        assertThat(details.get(NODES_OVER_FLOOD_STAGE_WATERMARK), equalTo(affectedNodes.size()));
+        assertThat(details.get(INDICES_WITH_READONLY_BLOCK), equalTo(0));
     }
 
     /*
@@ -339,11 +344,11 @@ public class DiskHealthIndicatorServiceTests extends ESTestCase {
         assertThat(affectedResources.get(0).getValues(), iterableWithSize(1));
 
         Map<String, Object> details = xContentToMap(result.details());
-        assertThat(details.get("green_nodes"), equalTo(discoveryNodes.size()));
-        assertThat(details.get("unknown_nodes"), equalTo(0));
-        assertThat(details.get("yellow_nodes"), equalTo(0));
-        assertThat(details.get("red_nodes"), equalTo(0));
-        assertThat(details.get("blocked_indices"), equalTo(1));
+        assertThat(details.get(NODES_WITH_ENOUGH_DISK_SPACE), equalTo(discoveryNodes.size()));
+        assertThat(details.get(NODES_WITH_UNKNOWN_DISK_STATUS), equalTo(0));
+        assertThat(details.get(NODES_OVER_HIGH_WATERMARK), equalTo(0));
+        assertThat(details.get(NODES_OVER_FLOOD_STAGE_WATERMARK), equalTo(0));
+        assertThat(details.get(INDICES_WITH_READONLY_BLOCK), equalTo(1));
     }
 
     /*
@@ -386,11 +391,11 @@ public class DiskHealthIndicatorServiceTests extends ESTestCase {
         assertThat(affectedResources.get(1).getType(), is(Diagnosis.Resource.Type.INDEX));
         assertThat(affectedResources.get(1).getValues(), iterableWithSize(1));
         Map<String, Object> details = xContentToMap(result.details());
-        assertThat(details.get("green_nodes"), equalTo(discoveryNodes.size() - numberOfYellowNodes));
-        assertThat(details.get("unknown_nodes"), equalTo(0));
-        assertThat(details.get("yellow_nodes"), equalTo(numberOfYellowNodes));
-        assertThat(details.get("red_nodes"), equalTo(0));
-        assertThat(details.get("blocked_indices"), equalTo(1));
+        assertThat(details.get(NODES_WITH_ENOUGH_DISK_SPACE), equalTo(discoveryNodes.size() - numberOfYellowNodes));
+        assertThat(details.get(NODES_WITH_UNKNOWN_DISK_STATUS), equalTo(0));
+        assertThat(details.get(NODES_OVER_HIGH_WATERMARK), equalTo(numberOfYellowNodes));
+        assertThat(details.get(NODES_OVER_FLOOD_STAGE_WATERMARK), equalTo(0));
+        assertThat(details.get(INDICES_WITH_READONLY_BLOCK), equalTo(1));
     }
 
     /*
@@ -447,11 +452,11 @@ public class DiskHealthIndicatorServiceTests extends ESTestCase {
             )
         );
         Map<String, Object> details = xContentToMap(result.details());
-        assertThat(details.get("green_nodes"), equalTo(discoveryNodes.size() - numberOfRedNodes));
-        assertThat(details.get("unknown_nodes"), equalTo(0));
-        assertThat(details.get("yellow_nodes"), equalTo(0));
-        assertThat(details.get("red_nodes"), equalTo(numberOfRedNodes));
-        assertThat(details.get("blocked_indices"), equalTo(blockedIndices.size()));
+        assertThat(details.get(NODES_WITH_ENOUGH_DISK_SPACE), equalTo(discoveryNodes.size() - numberOfRedNodes));
+        assertThat(details.get(NODES_WITH_UNKNOWN_DISK_STATUS), equalTo(0));
+        assertThat(details.get(NODES_OVER_HIGH_WATERMARK), equalTo(0));
+        assertThat(details.get(NODES_OVER_FLOOD_STAGE_WATERMARK), equalTo(numberOfRedNodes));
+        assertThat(details.get(INDICES_WITH_READONLY_BLOCK), equalTo(blockedIndices.size()));
     }
 
     public void testMissingHealthInfo() {