Skip to content

Commit

Permalink
Avoid negative DesiredBalanceStats#lastConvergedIndex (elastic#101998) (
Browse files Browse the repository at this point in the history
elastic#102001)

The initial state of the desired-balance allocator has
`lastConvergedIndex` set to `-1`. This is not important to represent in
the stats, so with this commit we map it to zero.
  • Loading branch information
DaveCTurner authored Nov 10, 2023
1 parent a8e9e86 commit fce89cb
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 1 deletion.
5 changes: 5 additions & 0 deletions docs/changelog/101998.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 101998
summary: Avoid negative `DesiredBalanceStats#lastConvergedIndex`
area: Allocation
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ public void resetDesiredBalance() {

public DesiredBalanceStats getStats() {
return new DesiredBalanceStats(
currentDesiredBalance.lastConvergedIndex(),
Math.max(currentDesiredBalance.lastConvergedIndex(), 0L),
desiredBalanceComputation.isActive(),
computationsSubmitted.count(),
computationsExecuted.count(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ public record DesiredBalanceStats(

private static final TransportVersion COMPUTED_SHARD_MOVEMENTS_VERSION = TransportVersions.V_8_8_0;

public DesiredBalanceStats {
if (lastConvergedIndex < 0) {
assert false : lastConvergedIndex;
throw new IllegalStateException("lastConvergedIndex must be nonnegative, but got [" + lastConvergedIndex + ']');
}
}

public static DesiredBalanceStats readFrom(StreamInput in) throws IOException {
return new DesiredBalanceStats(
in.readVLong(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import static org.elasticsearch.cluster.routing.UnassignedInfo.INDEX_DELAYED_NODE_LEFT_TIMEOUT_SETTING;
import static org.elasticsearch.common.settings.ClusterSettings.createBuiltInClusterSettings;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.not;

Expand Down Expand Up @@ -158,6 +159,7 @@ public ClusterState apply(ClusterState clusterState, RerouteStrategy routingAllo
clusterService,
reconcileAction
);
assertValidStats(desiredBalanceShardsAllocator.getStats());
var allocationService = createAllocationService(desiredBalanceShardsAllocator, createGatewayAllocator(allocateUnassigned));
allocationServiceRef.set(allocationService);

Expand Down Expand Up @@ -200,11 +202,21 @@ public void onFailure(Exception e) {
}
}
}
assertValidStats(desiredBalanceShardsAllocator.getStats());
} finally {
clusterService.close();
}
}

private void assertValidStats(DesiredBalanceStats stats) {
assertThat(stats.lastConvergedIndex(), greaterThanOrEqualTo(0L));
try {
assertEquals(stats, copyWriteable(stats, writableRegistry(), DesiredBalanceStats::readFrom));
} catch (Exception e) {
fail(e);
}
}

public void testShouldNotRemoveAllocationDelayMarkersOnReconcile() {

var localNode = newNode(LOCAL_NODE_ID);
Expand Down

0 comments on commit fce89cb

Please sign in to comment.