Skip to content

Commit fb47e81

Browse files
colings86colings86
committed
Revert "Fixes ByteSizeValue to serialise correctly (#27702)"
This reverts commit c355677.
1 parent c355677 commit fb47e81

File tree

10 files changed

+100
-314
lines changed

10 files changed

+100
-314
lines changed

‎core/src/main/java/org/elasticsearch/common/settings/Setting.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import java.util.function.BiConsumer;
5353
import java.util.function.Consumer;
5454
import java.util.function.Function;
55+
import java.util.function.IntConsumer;
5556
import java.util.regex.Matcher;
5657
import java.util.regex.Pattern;
5758
import java.util.stream.Collectors;
@@ -1019,7 +1020,7 @@ public static Setting<ByteSizeValue> byteSizeSetting(String key, Function<Settin
10191020

10201021
public static Setting<ByteSizeValue> byteSizeSetting(String key, ByteSizeValue defaultValue, ByteSizeValue minValue,
10211022
ByteSizeValue maxValue, Property... properties) {
1022-
return byteSizeSetting(key, (s) -> defaultValue.getStringRep(), minValue, maxValue, properties);
1023+
return byteSizeSetting(key, (s) -> defaultValue.toString(), minValue, maxValue, properties);
10231024
}
10241025

10251026
public static Setting<ByteSizeValue> byteSizeSetting(String key, Function<Settings, String> defaultValue,

‎core/src/main/java/org/elasticsearch/common/unit/ByteSizeUnit.java

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,6 @@ public long toTB(long size) {
6363
public long toPB(long size) {
6464
return size / (C5 / C0);
6565
}
66-
67-
@Override
68-
public String getSuffix() {
69-
return "b";
70-
}
7166
},
7267
KB {
7368
@Override
@@ -99,11 +94,6 @@ public long toTB(long size) {
9994
public long toPB(long size) {
10095
return size / (C5 / C1);
10196
}
102-
103-
@Override
104-
public String getSuffix() {
105-
return "kb";
106-
}
10797
},
10898
MB {
10999
@Override
@@ -135,11 +125,6 @@ public long toTB(long size) {
135125
public long toPB(long size) {
136126
return size / (C5 / C2);
137127
}
138-
139-
@Override
140-
public String getSuffix() {
141-
return "mb";
142-
}
143128
},
144129
GB {
145130
@Override
@@ -171,11 +156,6 @@ public long toTB(long size) {
171156
public long toPB(long size) {
172157
return size / (C5 / C3);
173158
}
174-
175-
@Override
176-
public String getSuffix() {
177-
return "gb";
178-
}
179159
},
180160
TB {
181161
@Override
@@ -207,11 +187,6 @@ public long toTB(long size) {
207187
public long toPB(long size) {
208188
return size / (C5 / C4);
209189
}
210-
211-
@Override
212-
public String getSuffix() {
213-
return "tb";
214-
}
215190
},
216191
PB {
217192
@Override
@@ -243,11 +218,6 @@ public long toTB(long size) {
243218
public long toPB(long size) {
244219
return size;
245220
}
246-
247-
@Override
248-
public String getSuffix() {
249-
return "pb";
250-
}
251221
};
252222

253223
static final long C0 = 1L;
@@ -288,8 +258,6 @@ static long x(long d, long m, long over) {
288258

289259
public abstract long toPB(long size);
290260

291-
public abstract String getSuffix();
292-
293261
@Override
294262
public void writeTo(StreamOutput out) throws IOException {
295263
out.writeVInt(this.ordinal());

‎core/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java

Lines changed: 51 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -20,71 +20,39 @@
2020
package org.elasticsearch.common.unit;
2121

2222
import org.elasticsearch.ElasticsearchParseException;
23-
import org.elasticsearch.Version;
2423
import org.elasticsearch.common.Strings;
2524
import org.elasticsearch.common.io.stream.StreamInput;
2625
import org.elasticsearch.common.io.stream.StreamOutput;
2726
import org.elasticsearch.common.io.stream.Writeable;
28-
import org.elasticsearch.common.logging.DeprecationLogger;
29-
import org.elasticsearch.common.logging.Loggers;
3027

3128
import java.io.IOException;
3229
import java.util.Locale;
3330
import java.util.Objects;
3431

3532
public class ByteSizeValue implements Writeable, Comparable<ByteSizeValue> {
36-
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ByteSizeValue.class));
3733

3834
private final long size;
3935
private final ByteSizeUnit unit;
4036

4137
public ByteSizeValue(StreamInput in) throws IOException {
42-
if (in.getVersion().before(Version.V_7_0_0_alpha1)) {
43-
size = in.readVLong();
44-
unit = ByteSizeUnit.BYTES;
45-
} else {
46-
size = in.readZLong();
47-
unit = ByteSizeUnit.readFrom(in);
48-
}
38+
size = in.readVLong();
39+
unit = ByteSizeUnit.BYTES;
4940
}
5041

5142
@Override
5243
public void writeTo(StreamOutput out) throws IOException {
53-
if (out.getVersion().before(Version.V_7_0_0_alpha1)) {
54-
out.writeVLong(getBytes());
55-
} else {
56-
out.writeZLong(size);
57-
unit.writeTo(out);
58-
}
44+
out.writeVLong(getBytes());
5945
}
6046

6147
public ByteSizeValue(long bytes) {
6248
this(bytes, ByteSizeUnit.BYTES);
6349
}
6450

6551
public ByteSizeValue(long size, ByteSizeUnit unit) {
66-
if (size < -1 || (size == -1 && unit != ByteSizeUnit.BYTES)) {
67-
throw new IllegalArgumentException("Values less than -1 bytes are not supported: " + size + unit.getSuffix());
68-
}
69-
if (size > Long.MAX_VALUE / unit.toBytes(1)) {
70-
throw new IllegalArgumentException(
71-
"Values greater than " + Long.MAX_VALUE + " bytes are not supported: " + size + unit.getSuffix());
72-
}
7352
this.size = size;
7453
this.unit = unit;
7554
}
7655

77-
// For testing
78-
long getSize() {
79-
return size;
80-
}
81-
82-
// For testing
83-
ByteSizeUnit getUnit() {
84-
return unit;
85-
}
86-
87-
@Deprecated
8856
public int bytesAsInt() {
8957
long bytes = getBytes();
9058
if (bytes > Integer.MAX_VALUE) {
@@ -137,41 +105,26 @@ public double getPbFrac() {
137105
return ((double) getBytes()) / ByteSizeUnit.C5;
138106
}
139107

140-
/**
141-
* @return a string representation of this value which is guaranteed to be
142-
* able to be parsed using
143-
* {@link #parseBytesSizeValue(String, ByteSizeValue, String)}.
144-
* Unlike {@link #toString()} this method will not output fractional
145-
* or rounded values so this method should be preferred when
146-
* serialising the value to JSON.
147-
*/
148-
public String getStringRep() {
149-
if (size <= 0) {
150-
return String.valueOf(size);
151-
}
152-
return size + unit.getSuffix();
153-
}
154-
155108
@Override
156109
public String toString() {
157110
long bytes = getBytes();
158111
double value = bytes;
159-
String suffix = ByteSizeUnit.BYTES.getSuffix();
112+
String suffix = "b";
160113
if (bytes >= ByteSizeUnit.C5) {
161114
value = getPbFrac();
162-
suffix = ByteSizeUnit.PB.getSuffix();
115+
suffix = "pb";
163116
} else if (bytes >= ByteSizeUnit.C4) {
164117
value = getTbFrac();
165-
suffix = ByteSizeUnit.TB.getSuffix();
118+
suffix = "tb";
166119
} else if (bytes >= ByteSizeUnit.C3) {
167120
value = getGbFrac();
168-
suffix = ByteSizeUnit.GB.getSuffix();
121+
suffix = "gb";
169122
} else if (bytes >= ByteSizeUnit.C2) {
170123
value = getMbFrac();
171-
suffix = ByteSizeUnit.MB.getSuffix();
124+
suffix = "mb";
172125
} else if (bytes >= ByteSizeUnit.C1) {
173126
value = getKbFrac();
174-
suffix = ByteSizeUnit.KB.getSuffix();
127+
suffix = "kb";
175128
}
176129
return Strings.format1Decimals(value, suffix);
177130
}
@@ -186,64 +139,47 @@ public static ByteSizeValue parseBytesSizeValue(String sValue, ByteSizeValue def
186139
if (sValue == null) {
187140
return defaultValue;
188141
}
189-
String lowerSValue = sValue.toLowerCase(Locale.ROOT).trim();
190-
if (lowerSValue.endsWith("k")) {
191-
return parse(sValue, lowerSValue, "k", ByteSizeUnit.KB, settingName);
192-
} else if (lowerSValue.endsWith("kb")) {
193-
return parse(sValue, lowerSValue, "kb", ByteSizeUnit.KB, settingName);
194-
} else if (lowerSValue.endsWith("m")) {
195-
return parse(sValue, lowerSValue, "m", ByteSizeUnit.MB, settingName);
196-
} else if (lowerSValue.endsWith("mb")) {
197-
return parse(sValue, lowerSValue, "mb", ByteSizeUnit.MB, settingName);
198-
} else if (lowerSValue.endsWith("g")) {
199-
return parse(sValue, lowerSValue, "g", ByteSizeUnit.GB, settingName);
200-
} else if (lowerSValue.endsWith("gb")) {
201-
return parse(sValue, lowerSValue, "gb", ByteSizeUnit.GB, settingName);
202-
} else if (lowerSValue.endsWith("t")) {
203-
return parse(sValue, lowerSValue, "t", ByteSizeUnit.TB, settingName);
204-
} else if (lowerSValue.endsWith("tb")) {
205-
return parse(sValue, lowerSValue, "tb", ByteSizeUnit.TB, settingName);
206-
} else if (lowerSValue.endsWith("p")) {
207-
return parse(sValue, lowerSValue, "p", ByteSizeUnit.PB, settingName);
208-
} else if (lowerSValue.endsWith("pb")) {
209-
return parse(sValue, lowerSValue, "pb", ByteSizeUnit.PB, settingName);
210-
} else if (lowerSValue.endsWith("b")) {
211-
return new ByteSizeValue(Long.parseLong(lowerSValue.substring(0, lowerSValue.length() - 1).trim()), ByteSizeUnit.BYTES);
212-
} else if (lowerSValue.equals("-1")) {
213-
// Allow this special value to be unit-less:
214-
return new ByteSizeValue(-1, ByteSizeUnit.BYTES);
215-
} else if (lowerSValue.equals("0")) {
216-
// Allow this special value to be unit-less:
217-
return new ByteSizeValue(0, ByteSizeUnit.BYTES);
218-
} else {
219-
// Missing units:
220-
throw new ElasticsearchParseException(
221-
"failed to parse setting [{}] with value [{}] as a size in bytes: unit is missing or unrecognized", settingName,
222-
sValue);
223-
}
224-
}
225-
226-
private static ByteSizeValue parse(final String initialInput, final String normalized, final String suffix, ByteSizeUnit unit,
227-
final String settingName) {
228-
final String s = normalized.substring(0, normalized.length() - suffix.length()).trim();
142+
long bytes;
229143
try {
230-
try {
231-
return new ByteSizeValue(Long.parseLong(s), unit);
232-
} catch (final NumberFormatException e) {
233-
try {
234-
final double doubleValue = Double.parseDouble(s);
235-
DEPRECATION_LOGGER.deprecated(
236-
"Fractional bytes values are deprecated. Use non-fractional bytes values instead: [{}] found for setting [{}]",
237-
initialInput, settingName);
238-
return new ByteSizeValue((long) (doubleValue * unit.toBytes(1)));
239-
} catch (final NumberFormatException ignored) {
240-
throw new ElasticsearchParseException("failed to parse [{}]", e, initialInput);
241-
}
144+
String lowerSValue = sValue.toLowerCase(Locale.ROOT).trim();
145+
if (lowerSValue.endsWith("k")) {
146+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 1)) * ByteSizeUnit.C1);
147+
} else if (lowerSValue.endsWith("kb")) {
148+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 2)) * ByteSizeUnit.C1);
149+
} else if (lowerSValue.endsWith("m")) {
150+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 1)) * ByteSizeUnit.C2);
151+
} else if (lowerSValue.endsWith("mb")) {
152+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 2)) * ByteSizeUnit.C2);
153+
} else if (lowerSValue.endsWith("g")) {
154+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 1)) * ByteSizeUnit.C3);
155+
} else if (lowerSValue.endsWith("gb")) {
156+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 2)) * ByteSizeUnit.C3);
157+
} else if (lowerSValue.endsWith("t")) {
158+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 1)) * ByteSizeUnit.C4);
159+
} else if (lowerSValue.endsWith("tb")) {
160+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 2)) * ByteSizeUnit.C4);
161+
} else if (lowerSValue.endsWith("p")) {
162+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 1)) * ByteSizeUnit.C5);
163+
} else if (lowerSValue.endsWith("pb")) {
164+
bytes = (long) (Double.parseDouble(lowerSValue.substring(0, lowerSValue.length() - 2)) * ByteSizeUnit.C5);
165+
} else if (lowerSValue.endsWith("b")) {
166+
bytes = Long.parseLong(lowerSValue.substring(0, lowerSValue.length() - 1).trim());
167+
} else if (lowerSValue.equals("-1")) {
168+
// Allow this special value to be unit-less:
169+
bytes = -1;
170+
} else if (lowerSValue.equals("0")) {
171+
// Allow this special value to be unit-less:
172+
bytes = 0;
173+
} else {
174+
// Missing units:
175+
throw new ElasticsearchParseException(
176+
"failed to parse setting [{}] with value [{}] as a size in bytes: unit is missing or unrecognized",
177+
settingName, sValue);
242178
}
243-
} catch (IllegalArgumentException e) {
244-
throw new ElasticsearchParseException("failed to parse setting [{}] with value [{}] as a size in bytes", e, settingName,
245-
initialInput);
179+
} catch (NumberFormatException e) {
180+
throw new ElasticsearchParseException("failed to parse [{}]", e, sValue);
246181
}
182+
return new ByteSizeValue(bytes, ByteSizeUnit.BYTES);
247183
}
248184

249185
@Override
@@ -260,13 +196,13 @@ public boolean equals(Object o) {
260196

261197
@Override
262198
public int hashCode() {
263-
return Long.hashCode(size * unit.toBytes(1));
199+
return Double.hashCode(((double) size) * unit.toBytes(1));
264200
}
265201

266202
@Override
267203
public int compareTo(ByteSizeValue other) {
268-
long thisValue = size * unit.toBytes(1);
269-
long otherValue = other.size * other.unit.toBytes(1);
270-
return Long.compare(thisValue, otherValue);
204+
double thisValue = ((double) size) * unit.toBytes(1);
205+
double otherValue = ((double) other.size) * other.unit.toBytes(1);
206+
return Double.compare(thisValue, otherValue);
271207
}
272208
}

‎core/src/test/java/org/elasticsearch/action/admin/cluster/node/stats/NodeStatsTests.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -311,11 +311,9 @@ private static NodeStats createNodeStats() {
311311
for (int i = 0; i < 3; i++) {
312312
loadAverages[i] = randomBoolean() ? randomDouble() : -1;
313313
}
314-
long memTotal = randomNonNegativeLong();
315-
long swapTotal = randomNonNegativeLong();
316314
osStats = new OsStats(System.currentTimeMillis(), new OsStats.Cpu(randomShort(), loadAverages),
317-
new OsStats.Mem(memTotal, randomLongBetween(0, memTotal)),
318-
new OsStats.Swap(swapTotal, randomLongBetween(0, swapTotal)),
315+
new OsStats.Mem(randomLong(), randomLong()),
316+
new OsStats.Swap(randomLong(), randomLong()),
319317
new OsStats.Cgroup(
320318
randomAlphaOfLength(8),
321319
randomNonNegativeLong(),

‎core/src/test/java/org/elasticsearch/cluster/DiskUsageTests.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,15 @@
2323
import org.elasticsearch.action.admin.cluster.node.stats.NodeStats;
2424
import org.elasticsearch.action.admin.indices.stats.CommonStats;
2525
import org.elasticsearch.action.admin.indices.stats.ShardStats;
26+
import org.elasticsearch.cluster.metadata.IndexMetaData;
27+
import org.elasticsearch.cluster.metadata.MetaData;
2628
import org.elasticsearch.cluster.node.DiscoveryNode;
2729
import org.elasticsearch.cluster.routing.RecoverySource.PeerRecoverySource;
2830
import org.elasticsearch.cluster.routing.ShardRouting;
2931
import org.elasticsearch.cluster.routing.ShardRoutingHelper;
3032
import org.elasticsearch.cluster.routing.UnassignedInfo;
3133
import org.elasticsearch.common.collect.ImmutableOpenMap;
34+
import org.elasticsearch.common.settings.Settings;
3235
import org.elasticsearch.index.Index;
3336
import org.elasticsearch.index.shard.ShardId;
3437
import org.elasticsearch.index.shard.ShardPath;
@@ -181,12 +184,12 @@ public void testFillDiskUsageSomeInvalidValues() {
181184
new FsInfo.Path("/most", "/dev/sdc", 300, 290, 280),
182185
};
183186
FsInfo.Path[] node2FSInfo = new FsInfo.Path[] {
184-
new FsInfo.Path("/least_most", "/dev/sda", -1, -1, -1),
187+
new FsInfo.Path("/least_most", "/dev/sda", -2, -1, -1),
185188
};
186189

187190
FsInfo.Path[] node3FSInfo = new FsInfo.Path[] {
188191
new FsInfo.Path("/most", "/dev/sda", 100, 90, 70),
189-
new FsInfo.Path("/least", "/dev/sda", 10, -1, 0),
192+
new FsInfo.Path("/least", "/dev/sda", 10, -8, 0),
190193
};
191194
List<NodeStats> nodeStats = Arrays.asList(
192195
new NodeStats(new DiscoveryNode("node_1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT), 0,

0 commit comments

Comments
 (0)