Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5c634a7
wip cluster info simulation
idegtiarenko Sep 14, 2022
342ba59
use cluster info simulator
idegtiarenko Sep 14, 2022
d0fa4c6
null check
idegtiarenko Sep 15, 2022
105fcef
Fix MockDiskUsagesIT
idegtiarenko Sep 15, 2022
7293edd
do not keep reserved info
idegtiarenko Sep 15, 2022
779016b
Merge branch 'feature/desired-balance-allocator' into simulate_disk_u…
idegtiarenko Sep 19, 2022
5c919f9
multiple data path test case
idegtiarenko Sep 20, 2022
103610a
add range check
idegtiarenko Sep 20, 2022
5aaad09
testComputeConsideringShardSizes
idegtiarenko Sep 21, 2022
cb7a174
keep node below watermark
idegtiarenko Sep 21, 2022
5faf34e
move class
idegtiarenko Sep 21, 2022
a8072ca
fix move
idegtiarenko Sep 21, 2022
2985353
rework test
idegtiarenko Sep 21, 2022
07be67f
Merge branch 'feature/desired-balance-allocator' into simulate_disk_u…
idegtiarenko Sep 21, 2022
480ad7a
Update server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java
idegtiarenko Sep 21, 2022
ee9a19f
Update server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java
idegtiarenko Sep 21, 2022
8c92948
make sure test is failing without simulated disk space
idegtiarenko Sep 21, 2022
2f1e6fb
Merge branch 'feature/desired-balance-allocator' into simulate_disk_u…
idegtiarenko Sep 26, 2022
67ee7dd
Merge branch 'feature/desired-balance-allocator' into simulate_disk_u…
idegtiarenko Sep 29, 2022
52fa7d3
Merge branch 'feature/desired-balance-allocator' into simulate_disk_u…
idegtiarenko Sep 29, 2022
93a72dc
Merge branch 'feature/desired-balance-allocator' into simulate_disk_u…
idegtiarenko Oct 3, 2022
b64afdd
Merge branch 'feature/desired-balance-allocator' into simulate_disk_u…
idegtiarenko Oct 4, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions server/src/main/java/org/elasticsearch/cluster/ClusterInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ public class ClusterInfo implements ToXContentFragment, Writeable {

private final Map<String, DiskUsage> leastAvailableSpaceUsage;
private final Map<String, DiskUsage> mostAvailableSpaceUsage;
final Map<String, Long> shardSizes;
final Map<ShardId, Long> shardDataSetSizes;
final Map<ShardRouting, String> routingToDataPath;
final Map<NodeAndPath, ReservedSpace> reservedSpace;
public final Map<String, Long> shardSizes;
public final Map<ShardId, Long> shardDataSetSizes;
public final Map<ShardRouting, String> routingToDataPath;
public final Map<NodeAndPath, ReservedSpace> reservedSpace;

protected ClusterInfo() {
this(Map.of(), Map.of(), Map.of(), Map.of(), Map.of(), Map.of());
Expand Down Expand Up @@ -224,7 +224,36 @@ public ReservedSpace getReservedSpace(String nodeId, String dataPath) {
* includes a 'p' or 'r' depending on whether the shard is a primary.
*/
public static String shardIdentifierFromRouting(ShardRouting shardRouting) {
return shardRouting.shardId().toString() + "[" + (shardRouting.primary() ? "p" : "r") + "]";
return shardIdentifierFromRouting(shardRouting.shardId(), shardRouting.primary());
}

public static String shardIdentifierFromRouting(ShardId shardId, boolean primary) {
return shardId.toString() + "[" + (primary ? "p" : "r") + "]";
}

@Override
public boolean equals(Object o) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use EqualsHashCodeTestUtils.checkEqualsAndHashCode to test this in ClusterInfoTests. Alternatively add a dedicated AbstractWireSerializingTestCase derivative for ClusterInfo which would cover equals and hashcode as well as replacing ClusterInfoTests#testSerialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the change to equals/hashCode and their tests be ported to main separately?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that'd be good.

if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ClusterInfo that = (ClusterInfo) o;
return leastAvailableSpaceUsage.equals(that.leastAvailableSpaceUsage)
&& mostAvailableSpaceUsage.equals(that.mostAvailableSpaceUsage)
&& shardSizes.equals(that.shardSizes)
&& shardDataSetSizes.equals(that.shardDataSetSizes)
&& routingToDataPath.equals(that.routingToDataPath)
&& reservedSpace.equals(that.reservedSpace);
}

@Override
public int hashCode() {
return Objects.hash(
leastAvailableSpaceUsage,
mostAvailableSpaceUsage,
shardSizes,
shardDataSetSizes,
routingToDataPath,
reservedSpace
);
}

@Override
Expand Down
4 changes: 4 additions & 0 deletions server/src/main/java/org/elasticsearch/cluster/DiskUsage.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ public String toString() {
+ "]";
}

public DiskUsage copyWithFreeBytes(long freeBytes) {
return new DiskUsage(nodeId, nodeName, path, totalBytes, freeBytes);
}

/**
* Finds the path with the least available disk space and returns its disk usage. It returns null if there is no
* file system data in the NodeStats or if the total bytes are a negative number.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class RoutingAllocation {

private final ClusterState clusterState;

private final ClusterInfo clusterInfo;
private ClusterInfo clusterInfo;

private final SnapshotShardSizeInfo shardSizeInfo;

Expand Down Expand Up @@ -367,6 +367,11 @@ public boolean isSimulating() {
return isSimulating;
}

public void setSimulatedClusterInfo(ClusterInfo clusterInfo) {
assert isSimulating : "Should be called only while simulating";
this.clusterInfo = clusterInfo;
}

public RoutingAllocation immutableClone() {
return new RoutingAllocation(deciders, clusterState, clusterInfo, shardSizeInfo, currentNanoTime);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.cluster.routing.allocation.allocator;

import org.elasticsearch.cluster.ClusterInfo;
import org.elasticsearch.cluster.DiskUsage;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.index.shard.ShardId;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

public class ClusterInfoSimulator {

private final Map<String, DiskUsage> leastAvailableSpaceUsage;
private final Map<String, DiskUsage> mostAvailableSpaceUsage;
private final Map<String, Long> shardSizes;
private final Map<ShardId, Long> shardDataSetSizes;
private final Map<ShardRouting, String> routingToDataPath;

public ClusterInfoSimulator(ClusterInfo clusterInfo) {
this.leastAvailableSpaceUsage = new HashMap<>(clusterInfo.getNodeLeastAvailableDiskUsages());
this.mostAvailableSpaceUsage = new HashMap<>(clusterInfo.getNodeMostAvailableDiskUsages());
this.shardSizes = new HashMap<>(clusterInfo.shardSizes);
this.shardDataSetSizes = new HashMap<>(clusterInfo.shardDataSetSizes);
this.routingToDataPath = new HashMap<>(clusterInfo.routingToDataPath);
}

public void simulate(ShardRouting shard) {
assert shard.initializing();

var size = getEstimatedShardSize(shard);
if (size != null && size > 0) {
if (shard.relocatingNodeId() != null) {
// relocation
modifyDiskUsage(shard.relocatingNodeId(), getShardPath(shard.relocatingNodeId(), mostAvailableSpaceUsage), size);
modifyDiskUsage(shard.currentNodeId(), getShardPath(shard.currentNodeId(), leastAvailableSpaceUsage), -size);
} else {
// new shard
modifyDiskUsage(shard.currentNodeId(), getShardPath(shard.currentNodeId(), leastAvailableSpaceUsage), -size);
shardSizes.put(ClusterInfo.shardIdentifierFromRouting(shard), size);
}
}
}

private Long getEstimatedShardSize(ShardRouting routing) {
if (routing.relocatingNodeId() != null) {
// relocation existing shard, get size of the source shard
return shardSizes.get(ClusterInfo.shardIdentifierFromRouting(routing));
} else if (routing.primary() == false) {
// initializing new replica, get size of the source primary shard
return shardSizes.get(ClusterInfo.shardIdentifierFromRouting(routing.shardId(), true));
} else {
// initializing new (empty) primary
return 0L;
}
}

private String getShardPath(String nodeId, Map<String, DiskUsage> defaultSpaceUsage) {
var diskUsage = defaultSpaceUsage.get(nodeId);
return diskUsage != null ? diskUsage.getPath() : null;
}

private void modifyDiskUsage(String nodeId, String path, long delta) {
var leastUsage = leastAvailableSpaceUsage.get(nodeId);
if (leastUsage != null && Objects.equals(leastUsage.getPath(), path)) {
// ensure new value is within bounds
leastAvailableSpaceUsage.put(nodeId, updateWithFreeBytes(leastUsage, delta));
}
var mostUsage = mostAvailableSpaceUsage.get(nodeId);
if (mostUsage != null && Objects.equals(mostUsage.getPath(), path)) {
// ensure new value is within bounds
mostAvailableSpaceUsage.put(nodeId, updateWithFreeBytes(mostUsage, delta));
}
}

private static DiskUsage updateWithFreeBytes(DiskUsage usage, long delta) {
// free bytes might go out of range in case when multiple data path are used
// we might not know exact disk used to allocate a shard and conservatively update
// most used disk on a target node and least used disk on a source node
var freeBytes = withinRange(0, usage.getTotalBytes(), usage.freeBytes() + delta);
return usage.copyWithFreeBytes(freeBytes);
}

private static long withinRange(long min, long max, long value) {
return Math.max(min, Math.min(max, value));
}

public ClusterInfo getClusterInfo() {
return new ClusterInfo(
leastAvailableSpaceUsage,
mostAvailableSpaceUsage,
shardSizes,
shardDataSetSizes,
routingToDataPath,
Map.of()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ public DesiredBalance compute(
final var routingNodes = routingAllocation.routingNodes();
final var ignoredShards = new HashSet<>(desiredBalanceInput.ignoredShards());
final var changes = routingAllocation.changes();
final var knownNodeIds = routingAllocation.nodes().stream().map(DiscoveryNode::getId).collect(Collectors.toSet());
final var unassignedPrimaries = new HashSet<ShardId>();
final var clusterInfoSimulator = new ClusterInfoSimulator(routingAllocation.clusterInfo());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we enhance DesiredBalanceComputerTests to verify we're accounting for disk usage in all the right places?


if (routingNodes.isEmpty()) {
return new DesiredBalance(desiredBalanceInput.index(), Map.of());
Expand All @@ -67,14 +66,15 @@ public DesiredBalance compute(
for (final var routingNode : routingNodes) {
for (final var shardRouting : routingNode) {
if (shardRouting.initializing()) {
clusterInfoSimulator.simulate(shardRouting);
routingNodes.startShard(logger, shardRouting, changes);
// TODO adjust disk usage info to reflect the assumed shard movement
}
}
}

// we are not responsible for allocating unassigned primaries of existing shards, and we're only responsible for allocating
// unassigned replicas if the ReplicaShardAllocator gives up, so we must respect these ignored shards
final var unassignedPrimaries = new HashSet<ShardId>();
final var shardRoutings = new HashMap<ShardId, ShardRoutings>();
for (final var primary : new boolean[] { true, false }) {
final RoutingNodes.UnassignedShards unassigned = routingNodes.unassigned();
Expand All @@ -98,6 +98,7 @@ public DesiredBalance compute(
}

// we can assume that all possible shards will be allocated/relocated to one of their desired locations
final var knownNodeIds = routingAllocation.nodes().stream().map(DiscoveryNode::getId).collect(Collectors.toSet());
final var unassignedShardsToInitialize = new HashMap<ShardRouting, LinkedList<String>>();
for (final var entry : shardRoutings.entrySet()) {
final var shardId = entry.getKey();
Expand Down Expand Up @@ -125,11 +126,9 @@ public DesiredBalance compute(
for (final var shardRouting : shardsToRelocate) {
assert shardRouting.started();
if (targetNodesIterator.hasNext()) {
routingNodes.startShard(
logger,
routingNodes.relocateShard(shardRouting, targetNodesIterator.next(), 0L, changes).v2(),
changes
);
ShardRouting shardToRelocate = routingNodes.relocateShard(shardRouting, targetNodesIterator.next(), 0L, changes).v2();
clusterInfoSimulator.simulate(shardToRelocate);
routingNodes.startShard(logger, shardToRelocate, changes);
} else {
break;
}
Expand All @@ -153,7 +152,9 @@ public DesiredBalance compute(
final var nodeIds = unassignedShardsToInitialize.get(shardRouting);
if (nodeIds != null && nodeIds.isEmpty() == false) {
final String nodeId = nodeIds.removeFirst();
routingNodes.startShard(logger, unassignedPrimaryIterator.initialize(nodeId, null, 0L, changes), changes);
ShardRouting shardToInitialized = unassignedPrimaryIterator.initialize(nodeId, null, 0L, changes);
clusterInfoSimulator.simulate(shardToInitialized);
routingNodes.startShard(logger, shardToInitialized, changes);
}
}
}
Expand All @@ -165,7 +166,9 @@ public DesiredBalance compute(
final var nodeIds = unassignedShardsToInitialize.get(shardRouting);
if (nodeIds != null && nodeIds.isEmpty() == false) {
final String nodeId = nodeIds.removeFirst();
routingNodes.startShard(logger, unassignedReplicaIterator.initialize(nodeId, null, 0L, changes), changes);
ShardRouting shardToInitialize = unassignedReplicaIterator.initialize(nodeId, null, 0L, changes);
clusterInfoSimulator.simulate(shardToInitialize);
routingNodes.startShard(logger, shardToInitialize, changes);
}
}
}
Expand Down Expand Up @@ -208,6 +211,7 @@ public DesiredBalance compute(
// TODO test that we reset ignored shards properly
}

routingAllocation.setSimulatedClusterInfo(clusterInfoSimulator.getClusterInfo());
logger.trace("running delegate allocator");
delegateAllocator.allocate(routingAllocation);
assert routingNodes.unassigned().size() == 0; // any unassigned shards should now be ignored
Expand All @@ -217,9 +221,9 @@ public DesiredBalance compute(
for (final var shardRouting : routingNode) {
if (shardRouting.initializing()) {
hasChanges = true;
clusterInfoSimulator.simulate(shardRouting);
routingNodes.startShard(logger, shardRouting, changes);
logger.trace("starting shard {}", shardRouting);
// TODO adjust disk usage info to reflect the assumed shard movement
}
}
}
Expand Down
Loading