Skip to content

Commit

Permalink
remove unnecessary getters (elastic#104117)
Browse files Browse the repository at this point in the history
  • Loading branch information
idegtiarenko authored Jan 18, 2024
1 parent 764269b commit 067eb34
Show file tree
Hide file tree
Showing 18 changed files with 113 additions and 134 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,11 @@ public void testClusterInfoServiceCollectsInformation() {
assertThat("some shard sizes are populated", shardSizes.values().size(), greaterThan(0));
for (DiskUsage usage : leastUsages.values()) {
logger.info("--> usage: {}", usage);
assertThat("usage has be retrieved", usage.getFreeBytes(), greaterThan(0L));
assertThat("usage has be retrieved", usage.freeBytes(), greaterThan(0L));
}
for (DiskUsage usage : mostUsages.values()) {
logger.info("--> usage: {}", usage);
assertThat("usage has be retrieved", usage.getFreeBytes(), greaterThan(0L));
assertThat("usage has be retrieved", usage.freeBytes(), greaterThan(0L));
}
for (Long size : shardSizes.values()) {
logger.info("--> shard size: {}", size);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ private void refreshDiskUsage() {
.getNodeMostAvailableDiskUsages()
.values()
.stream()
.allMatch(e -> e.getFreeBytes() > WATERMARK_BYTES)) {
.allMatch(e -> e.freeBytes() > WATERMARK_BYTES)) {
assertAcked(clusterAdmin().prepareReroute());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params params
return Iterators.concat(startObject("nodes"), Iterators.map(leastAvailableSpaceUsage.entrySet().iterator(), c -> (builder, p) -> {
builder.startObject(c.getKey());
{ // node
builder.field("node_name", c.getValue().getNodeName());
builder.field("node_name", c.getValue().nodeName());
builder.startObject("least_available");
{
c.getValue().toShortXContent(builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ private void modifyDiskUsage(String nodeId, long freeDelta) {
if (diskUsage == null) {
return;
}
var path = diskUsage.getPath();
var path = diskUsage.path();
updateDiskUsage(leastAvailableSpaceUsage, nodeId, path, freeDelta);
updateDiskUsage(mostAvailableSpaceUsage, nodeId, path, freeDelta);
}

private void updateDiskUsage(Map<String, DiskUsage> availableSpaceUsage, String nodeId, String path, long freeDelta) {
var usage = availableSpaceUsage.get(nodeId);
if (usage != null && Objects.equals(usage.getPath(), path)) {
if (usage != null && Objects.equals(usage.path(), path)) {
// ensure new value is within bounds
availableSpaceUsage.put(nodeId, updateWithFreeBytes(usage, freeDelta));
}
Expand All @@ -139,7 +139,7 @@ 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);
var freeBytes = withinRange(0, usage.totalBytes(), usage.freeBytes() + delta);
return usage.copyWithFreeBytes(freeBytes);
}

Expand Down
42 changes: 11 additions & 31 deletions server/src/main/java/org/elasticsearch/cluster/DiskUsage.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,33 +53,21 @@ private static double truncatePercent(double pct) {
XContentBuilder toShortXContent(XContentBuilder builder) throws IOException {
builder.field("path", this.path);
builder.humanReadableField("total_bytes", "total", ByteSizeValue.ofBytes(this.totalBytes));
builder.humanReadableField("used_bytes", "used", ByteSizeValue.ofBytes(this.getUsedBytes()));
builder.humanReadableField("used_bytes", "used", ByteSizeValue.ofBytes(this.usedBytes()));
builder.humanReadableField("free_bytes", "free", ByteSizeValue.ofBytes(this.freeBytes));
builder.field("free_disk_percent", truncatePercent(this.getFreeDiskAsPercentage()));
builder.field("used_disk_percent", truncatePercent(this.getUsedDiskAsPercentage()));
builder.field("free_disk_percent", truncatePercent(this.freeDiskAsPercentage()));
builder.field("used_disk_percent", truncatePercent(this.usedDiskAsPercentage()));
return builder;
}

public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.field("node_id", this.nodeId);
builder.field("node_name", this.nodeName);
builder = toShortXContent(builder);
toShortXContent(builder);
return builder;
}

public String getNodeId() {
return nodeId;
}

public String getNodeName() {
return nodeName;
}

public String getPath() {
return path;
}

public double getFreeDiskAsPercentage() {
public double freeDiskAsPercentage() {
// We return 100.0% in order to fail "open", in that if we have invalid
// numbers for the total bytes, it's as if we don't know disk usage.
if (totalBytes == 0) {
Expand All @@ -88,20 +76,12 @@ public double getFreeDiskAsPercentage() {
return 100.0 * freeBytes / totalBytes;
}

public double getUsedDiskAsPercentage() {
return 100.0 - getFreeDiskAsPercentage();
}

public long getFreeBytes() {
return freeBytes;
}

public long getTotalBytes() {
return totalBytes;
public double usedDiskAsPercentage() {
return 100.0 - freeDiskAsPercentage();
}

public long getUsedBytes() {
return getTotalBytes() - getFreeBytes();
public long usedBytes() {
return totalBytes - freeBytes;
}

@Override
Expand All @@ -113,9 +93,9 @@ public String toString() {
+ "]["
+ path
+ "] free: "
+ ByteSizeValue.ofBytes(getFreeBytes())
+ ByteSizeValue.ofBytes(this.freeBytes())
+ "["
+ Strings.format1Decimals(getFreeDiskAsPercentage(), "%")
+ Strings.format1Decimals(freeDiskAsPercentage(), "%")
+ "]";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,10 @@ public void onNewInfo(ClusterInfo info) {
final String node = entry.getKey();
final DiskUsage usage = entry.getValue();
final RoutingNode routingNode = routingNodes.node(node);
final ByteSizeValue total = ByteSizeValue.ofBytes(usage.getTotalBytes());
final ByteSizeValue total = ByteSizeValue.ofBytes(usage.totalBytes());

if (isDedicatedFrozenNode(routingNode)) {
if (usage.getFreeBytes() < diskThresholdSettings.getFreeBytesThresholdFrozenFloodStage(total).getBytes()) {
if (usage.freeBytes() < diskThresholdSettings.getFreeBytesThresholdFrozenFloodStage(total).getBytes()) {
logger.warn(
"flood stage disk watermark [{}] exceeded on {}",
diskThresholdSettings.describeFrozenFloodStageThreshold(total, false),
Expand All @@ -201,7 +201,7 @@ public void onNewInfo(ClusterInfo info) {
continue;
}

if (usage.getFreeBytes() < diskThresholdSettings.getFreeBytesThresholdFloodStage(total).getBytes()) {
if (usage.freeBytes() < diskThresholdSettings.getFreeBytesThresholdFloodStage(total).getBytes()) {
nodesOverLowThreshold.add(node);
nodesOverHighThreshold.add(node);
nodesOverHighThresholdAndRelocating.remove(node);
Expand All @@ -223,7 +223,7 @@ public void onNewInfo(ClusterInfo info) {
continue;
}

if (usage.getFreeBytes() < diskThresholdSettings.getFreeBytesThresholdHighStage(total).getBytes()) {
if (usage.freeBytes() < diskThresholdSettings.getFreeBytesThresholdHighStage(total).getBytes()) {
if (routingNode != null) { // might be temporarily null if the ClusterInfoService and the ClusterService are out of step
for (ShardRouting routing : routingNode) {
String indexName = routing.index().getName();
Expand All @@ -232,16 +232,16 @@ public void onNewInfo(ClusterInfo info) {
}
}

final long reservedSpace = info.getReservedSpace(usage.getNodeId(), usage.getPath()).total();
final long reservedSpace = info.getReservedSpace(usage.nodeId(), usage.path()).total();
final DiskUsage usageWithReservedSpace = new DiskUsage(
usage.getNodeId(),
usage.getNodeName(),
usage.getPath(),
usage.getTotalBytes(),
Math.max(0L, usage.getFreeBytes() - reservedSpace)
usage.nodeId(),
usage.nodeName(),
usage.path(),
usage.totalBytes(),
Math.max(0L, usage.freeBytes() - reservedSpace)
);

if (usageWithReservedSpace.getFreeBytes() < diskThresholdSettings.getFreeBytesThresholdHighStage(total).getBytes()) {
if (usageWithReservedSpace.freeBytes() < diskThresholdSettings.getFreeBytesThresholdHighStage(total).getBytes()) {
nodesOverLowThreshold.add(node);
nodesOverHighThreshold.add(node);

Expand All @@ -258,7 +258,7 @@ public void onNewInfo(ClusterInfo info) {
);
}

} else if (usageWithReservedSpace.getFreeBytes() < diskThresholdSettings.getFreeBytesThresholdLowStage(total).getBytes()) {
} else if (usageWithReservedSpace.freeBytes() < diskThresholdSettings.getFreeBytesThresholdLowStage(total).getBytes()) {
nodesOverHighThresholdAndRelocating.remove(node);

final boolean wasUnderLowThreshold = nodesOverLowThreshold.add(node);
Expand Down Expand Up @@ -321,33 +321,33 @@ public void onNewInfo(ClusterInfo info) {
ActionListener.releaseAfter(ActionListener.runAfter(ActionListener.wrap(ignored -> {
final var reroutedClusterState = clusterStateSupplier.get();
for (DiskUsage diskUsage : usagesOverHighThreshold) {
final RoutingNode routingNode = reroutedClusterState.getRoutingNodes().node(diskUsage.getNodeId());
final RoutingNode routingNode = reroutedClusterState.getRoutingNodes().node(diskUsage.nodeId());
final DiskUsage usageIncludingRelocations;
final long relocatingShardsSize;
if (routingNode != null) { // might be temporarily null if ClusterInfoService and ClusterService are out of step
relocatingShardsSize = sizeOfRelocatingShards(routingNode, diskUsage, info, reroutedClusterState);
usageIncludingRelocations = new DiskUsage(
diskUsage.getNodeId(),
diskUsage.getNodeName(),
diskUsage.getPath(),
diskUsage.getTotalBytes(),
diskUsage.getFreeBytes() - relocatingShardsSize
diskUsage.nodeId(),
diskUsage.nodeName(),
diskUsage.path(),
diskUsage.totalBytes(),
diskUsage.freeBytes() - relocatingShardsSize
);
} else {
usageIncludingRelocations = diskUsage;
relocatingShardsSize = 0L;
}
final ByteSizeValue total = ByteSizeValue.ofBytes(usageIncludingRelocations.getTotalBytes());
final ByteSizeValue total = ByteSizeValue.ofBytes(usageIncludingRelocations.totalBytes());

if (usageIncludingRelocations.getFreeBytes() < diskThresholdSettings.getFreeBytesThresholdHighStage(total)
if (usageIncludingRelocations.freeBytes() < diskThresholdSettings.getFreeBytesThresholdHighStage(total)
.getBytes()) {
nodesOverHighThresholdAndRelocating.remove(diskUsage.getNodeId());
nodesOverHighThresholdAndRelocating.remove(diskUsage.nodeId());
logger.warn("""
high disk watermark [{}] exceeded on {}, shards will be relocated away from this node; currently \
relocating away shards totalling [{}] bytes; the node is expected to continue to exceed the high disk \
watermark when these relocations are complete\
""", diskThresholdSettings.describeHighThreshold(total, false), diskUsage, -relocatingShardsSize);
} else if (nodesOverHighThresholdAndRelocating.add(diskUsage.getNodeId())) {
} else if (nodesOverHighThresholdAndRelocating.add(diskUsage.nodeId())) {
logger.info("""
high disk watermark [{}] exceeded on {}, shards will be relocated away from this node; currently \
relocating away shards totalling [{}] bytes; the node is expected to be below the high disk watermark \
Expand Down Expand Up @@ -424,7 +424,7 @@ long sizeOfRelocatingShards(RoutingNode routingNode, DiskUsage diskUsage, Cluste
return DiskThresholdDecider.sizeOfUnaccountedShards(
routingNode,
true,
diskUsage.getPath(),
diskUsage.path(),
info,
SnapshotShardSizeInfo.EMPTY,
reroutedClusterState.metadata(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ private static Map<String, Long> unaccountedSearchableSnapshotSizes(ClusterState
if (clusterInfo != null) {
for (RoutingNode node : clusterState.getRoutingNodes()) {
DiskUsage usage = clusterInfo.getNodeMostAvailableDiskUsages().get(node.nodeId());
ClusterInfo.ReservedSpace reservedSpace = clusterInfo.getReservedSpace(node.nodeId(), usage != null ? usage.getPath() : "");
ClusterInfo.ReservedSpace reservedSpace = clusterInfo.getReservedSpace(node.nodeId(), usage != null ? usage.path() : "");
long totalSize = 0;
for (ShardRouting shard : node.started()) {
if (shard.getExpectedShardSize() > 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,9 @@ private static DiskUsageWithRelocations getDiskUsage(
logger.debug(
"unable to determine disk usage for {}, defaulting to average across nodes [{} total] [{} free] [{}% free]",
node.nodeId(),
usage.getTotalBytes(),
usage.getFreeBytes(),
usage.getFreeDiskAsPercentage()
usage.totalBytes(),
usage.freeBytes(),
usage.freeDiskAsPercentage()
);
}

Expand All @@ -483,7 +483,7 @@ private static DiskUsageWithRelocations getDiskUsage(
sizeOfUnaccountedShards(
node,
subtractLeavingShards,
usage.getPath(),
usage.path(),
allocation.clusterInfo(),
allocation.snapshotShardSizeInfo(),
allocation.metadata(),
Expand All @@ -509,8 +509,8 @@ static DiskUsage averageUsage(RoutingNode node, Map<String, DiskUsage> usages) {
long totalBytes = 0;
long freeBytes = 0;
for (DiskUsage du : usages.values()) {
totalBytes += du.getTotalBytes();
freeBytes += du.getFreeBytes();
totalBytes += du.totalBytes();
freeBytes += du.freeBytes();
}
return new DiskUsage(node.nodeId(), node.node().getName(), "_na_", totalBytes / usages.size(), freeBytes / usages.size());
}
Expand Down Expand Up @@ -548,18 +548,18 @@ record DiskUsageWithRelocations(DiskUsage diskUsage, long relocatingShardSize) {

long getFreeBytes() {
try {
return Math.subtractExact(diskUsage.getFreeBytes(), relocatingShardSize);
return Math.subtractExact(diskUsage.freeBytes(), relocatingShardSize);
} catch (ArithmeticException e) {
return Long.MAX_VALUE;
}
}

String getPath() {
return diskUsage.getPath();
return diskUsage.path();
}

long getTotalBytes() {
return diskUsage.getTotalBytes();
return diskUsage.totalBytes();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -432,24 +432,24 @@ DiskHealthInfo getHealth(HealthMetadata healthMetadata, ClusterState clusterStat
return new DiskHealthInfo(HealthStatus.UNKNOWN, DiskHealthInfo.Cause.NODE_HAS_NO_DISK_STATS);
}

ByteSizeValue totalBytes = ByteSizeValue.ofBytes(usage.getTotalBytes());
ByteSizeValue totalBytes = ByteSizeValue.ofBytes(usage.totalBytes());

if (node.isDedicatedFrozenNode() || isDedicatedSearchNode(node)) {
long frozenFloodStageThreshold = diskMetadata.getFreeBytesFrozenFloodStageWatermark(totalBytes).getBytes();
if (usage.getFreeBytes() < frozenFloodStageThreshold) {
if (usage.freeBytes() < frozenFloodStageThreshold) {
logger.debug("Flood stage disk watermark [{}] exceeded on {}", frozenFloodStageThreshold, usage);
return new DiskHealthInfo(HealthStatus.RED, DiskHealthInfo.Cause.FROZEN_NODE_OVER_FLOOD_STAGE_THRESHOLD);
}
return new DiskHealthInfo(HealthStatus.GREEN);
}
long floodStageThreshold = diskMetadata.getFreeBytesFloodStageWatermark(totalBytes).getBytes();
if (usage.getFreeBytes() < floodStageThreshold) {
if (usage.freeBytes() < floodStageThreshold) {
logger.debug("Flood stage disk watermark [{}] exceeded on {}", floodStageThreshold, usage);
return new DiskHealthInfo(HealthStatus.RED, DiskHealthInfo.Cause.NODE_OVER_THE_FLOOD_STAGE_THRESHOLD);
}

long highThreshold = diskMetadata.getFreeBytesHighWatermark(totalBytes).getBytes();
if (usage.getFreeBytes() < highThreshold) {
if (usage.freeBytes() < highThreshold) {
if (node.canContainData()) {
// for data nodes only report YELLOW if shards can't move away from the node
if (DiskCheck.hasRelocatingShards(clusterState, node) == false) {
Expand Down
Loading

0 comments on commit 067eb34

Please sign in to comment.