-
Couldn't load subscription status.
- Fork 25.6k
Avoid NPE in DiskThresholdMonitor #93699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid NPE in DiskThresholdMonitor #93699
Conversation
| Hi @pxsalehi, I've created a changelog YAML for you. |
| Pinging @elastic/es-distributed (Team:Distributed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes look good but I think we should have some tests too - see DiskThresholdMonitorTests
…pxsalehi/elasticsearch into ps230210-avoidNPEInDiskThresholdMonitor
…DiskThresholdMonitor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a couple of very small nits
| assertEquals(Set.of("test"), result.v2()); | ||
| | ||
| final ClusterState blockedClusterState = ClusterState.builder(clusterState) | ||
| .blocks(ClusterBlocks.builder().addGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK).build()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we should also have an empty routing table in this state (the routing table is initialised when removing GatewayService.STATE_NOT_RECOVERED_BLOCK)
| DiscoveryNodes.Builder discoveryNodes = DiscoveryNodes.builder() | ||
| .add(newNormalNode("node1", "node1")) | ||
| .add(newNormalNode("node2", "node2")); | ||
| // node3 which is to replace node1 may or may not bee in the cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo :)
| // node3 which is to replace node1 may or may not bee in the cluster | |
| // node3 which is to replace node1 may or may not be in the cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐝
- Process new ClusterInfo only when the cluster state is recovered, since we need to know the indices/nodes in the cluster for this. - Avoid missing node IDs that could result from a REPLACE shutdown metadata. Both of these could lead to NPEs which is not fatal in 8.7/8.8, but could lead to unresolved listeners on 7.17.
💔 Backport failed
You can use sqren/backport to manually backport by running |
- Process new ClusterInfo only when the cluster state is recovered, since we need to know the indices/nodes in the cluster for this. - Avoid missing node IDs that could result from a REPLACE shutdown metadata. Both of these could lead to NPEs which is not fatal in 8.7/8.8, but could lead to unresolved listeners on 7.17.
- Process new ClusterInfo only when the cluster state is recovered, since we need to know the indices/nodes in the cluster for this. - Avoid missing node IDs that could result from a REPLACE shutdown metadata. Both of these could lead to NPEs which is not fatal in 8.7/8.8, but could lead to unresolved listeners on 7.17.
* Fix Gradle project evaluation when runtime java home is unset * Avoid NPE in DiskThresholdMonitor (#93699) - Process new ClusterInfo only when the cluster state is recovered, since we need to know the indices/nodes in the cluster for this. - Avoid missing node IDs that could result from a REPLACE shutdown metadata. Both of these could lead to NPEs which is not fatal in 8.7/8.8, but could lead to unresolved listeners on 7.17. --------- Co-authored-by: Mark Vieira <portugee@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
- Process new ClusterInfo only when the cluster state is recovered, since we need to know the indices/nodes in the cluster for this. - Avoid missing node IDs that could result from a REPLACE shutdown metadata. Both of these could lead to NPEs which is not fatal in 8.7/8.8, but could lead to unresolved listeners on 7.17.
ClusterInfoonly when the cluster state is recovered, since we need to know the indices/nodes in the cluster for this.REPLACEshutdown metadata.Both of these could lead to NPEs which is not fatal in 8.7/8.8, but could lead to unresolved listeners on 7.17.