Skip to content

Commit 7742eb9

Browse files
berndthll
bernd
and
thll
authored
Add Tools.percentageOf method (#24552)
We used several hand-rolled percentage computations in our code base. This change refactors all of them to use the tool method. --------- Co-authored-by: Othello Maurer <othello@graylog.com>
1 parent a89b872 commit 7742eb9

File tree

10 files changed

+179
-30
lines changed

10 files changed

+179
-30
lines changed

‎data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/SearchableSnapshotsConfigurationBean.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.graylog.datanode.process.configuration.beans.OpensearchKeystoreItem;
3737
import org.graylog.datanode.process.configuration.beans.OpensearchKeystoreStringItem;
3838
import org.graylog2.bootstrap.preflight.PreflightCheckException;
39+
import org.graylog2.plugin.Tools;
3940
import org.slf4j.Logger;
4041
import org.slf4j.LoggerFactory;
4142

@@ -138,7 +139,7 @@ private void validateUsableSpace() throws OpensearchConfigurationException {
138139
}
139140

140141
private double percentageUsage(long usableSpace, long cacheSize) {
141-
return 100.0 / usableSpace * cacheSize;
142+
return Tools.percentageOf(usableSpace, cacheSize);
142143
}
143144

144145
@Nonnull

‎graylog2-server/src/main/java/org/graylog2/indexer/indices/jobs/IndexSetCleanupJob.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,16 @@
1717
package org.graylog2.indexer.indices.jobs;
1818

1919
import com.google.inject.assistedinject.Assisted;
20+
import jakarta.inject.Inject;
2021
import org.graylog2.indexer.indexset.IndexSet;
2122
import org.graylog2.indexer.indexset.IndexSetConfig;
2223
import org.graylog2.indexer.indices.Indices;
2324
import org.graylog2.indexer.ranges.MongoIndexRangeService;
25+
import org.graylog2.plugin.Tools;
2426
import org.graylog2.system.jobs.SystemJob;
2527
import org.slf4j.Logger;
2628
import org.slf4j.LoggerFactory;
2729

28-
import jakarta.inject.Inject;
29-
3030
import java.util.concurrent.atomic.AtomicLong;
3131

3232
public class IndexSetCleanupJob extends SystemJob {
@@ -95,7 +95,7 @@ public int getProgress() {
9595
if (total <= 0) {
9696
return 0;
9797
}
98-
return (int) Math.floor((deleted.floatValue() / (float) total) * 100);
98+
return Tools.percentageOfRounded(total, deleted.intValue());
9999
}
100100

101101
@Override

‎graylog2-server/src/main/java/org/graylog2/indexer/ranges/RebuildIndexRangesJob.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.graylog2.indexer.indexset.IndexSet;
2626
import org.graylog2.indexer.indexset.basic.BasicIndexSet;
2727
import org.graylog2.indexer.indices.TooManyAliasesException;
28+
import org.graylog2.plugin.Tools;
2829
import org.graylog2.shared.system.activities.Activity;
2930
import org.graylog2.shared.system.activities.ActivityWriter;
3031
import org.graylog2.system.jobs.SystemJob;
@@ -71,8 +72,7 @@ public int getProgress() {
7172
return 0;
7273
}
7374

74-
// lolwtfbbqcasting
75-
return (int) Math.floor((indicesCalculated.floatValue() / (float) indicesToCalculate) * 100);
75+
return Tools.percentageOfRounded(indicesToCalculate, indicesCalculated.intValue());
7676
}
7777

7878
@Override

‎graylog2-server/src/main/java/org/graylog2/plugin/Tools.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,4 +674,42 @@ public static Optional<AbsoluteRange> extractHistogramBoundaries(final String qu
674674
public static int availableProcessors() {
675675
return AVAILABLE_PROCESSORS;
676676
}
677+
678+
/**
679+
* Calculate percentage from total and value, returning a double for precision.
680+
*
681+
* @param total The total number of items
682+
* @param value The value to calculate percentage for
683+
* @return A double between 0.0 and 100.0 representing the percentage.
684+
* Returns 0.0 if total is 0 or negative, or if value is 0 or negative.
685+
* Returns 100.0 if value is greater than or equal to total.
686+
*/
687+
public static double percentageOf(long total, long value) {
688+
// Handle invalid inputs
689+
if (total <= 0 || value <= 0) {
690+
return 0.0;
691+
}
692+
693+
// Cap at 100% if value exceeds total
694+
if (value >= total) {
695+
return 100.0;
696+
}
697+
698+
// Calculate percentage with full precision
699+
return (double) value * 100.0 / (double) total;
700+
}
701+
702+
/**
703+
* Calculate percentage from total and value, returning rounded integer.
704+
* Uses Math.round() for banker's rounding instead of floor.
705+
*
706+
* @param total The total number of items
707+
* @param value The value to calculate percentage for
708+
* @return An integer between 0 and 100 representing the percentage.
709+
* Returns 0 if total is 0 or negative, or if value is 0 or negative.
710+
* Returns 100 if value is greater than or equal to total.
711+
*/
712+
public static int percentageOfRounded(long total, long value) {
713+
return Math.toIntExact(Math.round(percentageOf(total, value)));
714+
}
677715
}

‎graylog2-server/src/main/java/org/graylog2/plugin/inputs/transports/ThrottleableTransport.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.common.eventbus.EventBus;
2020
import com.google.common.eventbus.Subscribe;
2121
import org.graylog2.plugin.ThrottleState;
22+
import org.graylog2.plugin.Tools;
2223
import org.graylog2.plugin.configuration.Configuration;
2324
import org.graylog2.plugin.configuration.ConfigurationRequest;
2425
import org.graylog2.plugin.configuration.fields.BooleanField;
@@ -200,12 +201,12 @@ protected boolean determineIfThrottled(ThrottleState state) {
200201
log.debug("[{}] [unthrottled] no incoming messages and nothing read from journal even if we could", transportName);
201202
return false;
202203
}
203-
if ((state.journalSize / (double) state.journalSizeLimit) * 100.0 > 90) {
204+
if (Tools.percentageOf(state.journalSizeLimit, state.journalSize) > 90) {
204205
// more than 90% of the journal limit is in use, don't read more if possible to avoid throwing away data
205206
log.debug("[{}] [throttled] journal more than 90% full", transportName);
206207
return true;
207208
}
208-
if ((state.readEventsPerSec / (double) state.appendEventsPerSec) * 100.0 < 50) {
209+
if (Tools.percentageOf(state.appendEventsPerSec, state.readEventsPerSec) < 50) {
209210
// read rate is less than 50% of what we write to the journal over the last second, let's try to back off
210211
log.debug("[{}] [throttled] write rate is more than twice as high than read rate", transportName);
211212
return true;

‎graylog2-server/src/main/java/org/graylog2/shared/journal/LocalKafkaJournal.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import org.graylog2.plugin.GlobalMetricNames;
6060
import org.graylog2.plugin.ServerStatus;
6161
import org.graylog2.plugin.ThrottleState;
62+
import org.graylog2.plugin.Tools;
6263
import org.graylog2.plugin.lifecycles.LoadBalancerStatus;
6364
import org.graylog2.shared.metrics.HdrTimer;
6465
import org.graylog2.shared.utilities.ByteBufferUtils;
@@ -855,7 +856,7 @@ public Optional<Double> getJournalUtilization() {
855856
* @return an {@code Optional<Double>} containing the journal utilization as a percentage.
856857
*/
857858
private double calculateUtilization(long maxRetentionSize, long kafkaLogSize) {
858-
return maxRetentionSize > 0 ? (double) (kafkaLogSize * 100) / maxRetentionSize : 0.0;
859+
return Tools.percentageOf(maxRetentionSize, kafkaLogSize);
859860
}
860861

861862
@Override

‎graylog2-server/src/main/java/org/graylog2/shared/system/stats/fs/JmxFsProbe.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@
1717
package org.graylog2.shared.system.stats.fs;
1818

1919
import com.google.common.collect.ImmutableSet;
20+
import com.google.common.primitives.Shorts;
21+
import jakarta.inject.Inject;
2022
import org.graylog2.Configuration;
2123
import org.graylog2.plugin.KafkaJournalConfiguration;
22-
23-
import jakarta.inject.Inject;
24+
import org.graylog2.plugin.Tools;
2425

2526
import java.io.File;
2627
import java.util.HashMap;
@@ -49,7 +50,7 @@ public FsStats fsStats() {
4950
final long free = location.getFreeSpace();
5051
final long available = location.getUsableSpace();
5152
final long used = total - free;
52-
final short usedPercent = (short) ((double) used / total * 100);
53+
final short usedPercent = Shorts.saturatedCast(Tools.percentageOfRounded(total, used));
5354

5455
final FsStats.Filesystem filesystem = FsStats.Filesystem.create(
5556
path, total, free, available, used, usedPercent);

‎graylog2-server/src/main/java/org/graylog2/shared/system/stats/fs/OshiFsProbe.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@
1717
package org.graylog2.shared.system.stats.fs;
1818

1919
import com.google.common.collect.ImmutableSet;
20+
import com.google.common.primitives.Shorts;
21+
import jakarta.inject.Inject;
2022
import org.apache.commons.lang3.StringUtils;
2123
import org.graylog2.Configuration;
2224
import org.graylog2.plugin.KafkaJournalConfiguration;
25+
import org.graylog2.plugin.Tools;
2326
import org.graylog2.shared.system.stats.OshiService;
2427
import org.slf4j.Logger;
2528
import org.slf4j.LoggerFactory;
@@ -33,8 +36,6 @@
3336
import oshi.software.os.OperatingSystem;
3437
import oshi.util.tuples.Pair;
3538

36-
import jakarta.inject.Inject;
37-
3839
import java.nio.file.Path;
3940
import java.nio.file.Paths;
4041
import java.util.Comparator;
@@ -120,9 +121,9 @@ public FsStats fsStats() {
120121
Optional.ofNullable(fileStore.getLogicalVolume()).orElse(fileStore.getVolume()),
121122
fileStore.getDescription(), fileStore.getType(), fileStore.getTotalSpace(), fileStore.getUsableSpace(),
122123
fileStore.getUsableSpace(), fileStore.getTotalSpace() - fileStore.getUsableSpace(),
123-
safePercentage(fileStore.getTotalSpace() - fileStore.getUsableSpace(), fileStore.getTotalSpace(), 0),
124+
Shorts.saturatedCast(Tools.percentageOfRounded(fileStore.getTotalSpace(), fileStore.getTotalSpace() - fileStore.getUsableSpace())),
124125
fileStore.getTotalInodes(), fileStore.getFreeInodes(), fileStore.getTotalInodes() - fileStore.getFreeInodes(),
125-
safePercentage(fileStore.getTotalInodes() - fileStore.getFreeInodes(), fileStore.getTotalInodes(), 0),
126+
Shorts.saturatedCast(Tools.percentageOfRounded(fileStore.getTotalInodes(), fileStore.getTotalInodes() - fileStore.getFreeInodes())),
126127
diskStore.getReads(),
127128
diskStore.getWrites(),
128129
diskStore.getReadBytes(),
@@ -134,10 +135,6 @@ public FsStats fsStats() {
134135
return FsStats.create(fsmap);
135136
}
136137

137-
private short safePercentage(long nominator, long denominator, int override) {
138-
return (denominator == 0) ? (short) override : (short) (nominator * 100 / denominator);
139-
}
140-
141138
private HWDiskStore generateDummyDiskStore() {
142139
return new AbstractHWDiskStore("missing", "missing", "missing", 0L) {
143140
@Override

‎graylog2-server/src/main/java/org/graylog2/shared/system/stats/os/OshiOsProbe.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616
*/
1717
package org.graylog2.shared.system.stats.os;
1818

19+
import com.google.common.primitives.Shorts;
20+
import jakarta.inject.Inject;
21+
import jakarta.inject.Singleton;
22+
import org.graylog2.plugin.Tools;
1923
import org.graylog2.shared.system.stats.OshiService;
2024
import oshi.hardware.CentralProcessor;
2125
import oshi.hardware.CentralProcessor.TickType;
@@ -24,9 +28,6 @@
2428
import oshi.hardware.VirtualMemory;
2529
import oshi.util.Util;
2630

27-
import jakarta.inject.Inject;
28-
import jakarta.inject.Singleton;
29-
3031
@Singleton
3132
public class OshiOsProbe implements OsProbe {
3233

@@ -47,9 +48,9 @@ public OsStats osStats() {
4748
final Memory mem = Memory.create(
4849
globalMemory.getTotal(),
4950
globalMemory.getAvailable(),
50-
safePercentage(globalMemory.getAvailable(), globalMemory.getTotal(), 0),
51+
Shorts.saturatedCast(Tools.percentageOfRounded(globalMemory.getTotal(), globalMemory.getAvailable())),
5152
globalMemory.getTotal() - globalMemory.getAvailable(),
52-
safePercentage(globalMemory.getTotal() - globalMemory.getAvailable(), globalMemory.getTotal(), 0),
53+
Shorts.saturatedCast(Tools.percentageOfRounded(globalMemory.getTotal(), globalMemory.getTotal() - globalMemory.getAvailable())),
5354
globalMemory.getAvailable(),
5455
globalMemory.getTotal() - globalMemory.getAvailable());
5556

@@ -106,7 +107,4 @@ public OsStats osStats() {
106107
swap);
107108
}
108109

109-
private short safePercentage(long nominator, long denominator, int override) {
110-
return (denominator == 0) ? (short) override : (short) (nominator * 100 / denominator);
111-
}
112110
}

‎graylog2-server/src/test/java/org/graylog2/plugin/ToolsTest.java‎

Lines changed: 114 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,12 +147,12 @@ public void testDecompressGzipBomb() throws URISyntaxException, IOException {
147147
@Test
148148
public void testDecompressGzipEmptyInput() throws IOException {
149149
assertThrows(EOFException.class, () ->
150-
Tools.decompressGzip(new byte[0]));
150+
Tools.decompressGzip(new byte[0]));
151151
}
152152

153153
/**
154154
* ruby-1.9.2-p136 :001 > [Time.now.to_i, 2.days.ago.to_i]
155-
* => [1322063329, 1321890529]
155+
* => [1322063329, 1321890529]
156156
*/
157157
@Test
158158
public void testGetTimestampDaysAgo() {
@@ -337,4 +337,116 @@ public void isWildcardAddress() {
337337
assertFalse(Tools.isWildcardInetAddress(InetAddresses.forString("198.51.100.23")));
338338
assertFalse(Tools.isWildcardInetAddress(InetAddresses.forString("2001:DB8::42")));
339339
}
340+
341+
@Test
342+
public void percentageOfRoundedNormalCases() {
343+
assertEquals(0, Tools.percentageOfRounded(100, 0));
344+
assertEquals(1, Tools.percentageOfRounded(100, 1));
345+
assertEquals(50, Tools.percentageOfRounded(100, 50));
346+
assertEquals(99, Tools.percentageOfRounded(100, 99));
347+
assertEquals(100, Tools.percentageOfRounded(100, 100));
348+
assertEquals(25, Tools.percentageOfRounded(200, 50));
349+
assertEquals(33, Tools.percentageOfRounded(300, 100));
350+
}
351+
352+
@Test
353+
public void percentageOfRoundedWithZeroTotal() {
354+
assertEquals(0, Tools.percentageOfRounded(0, 0));
355+
assertEquals(0, Tools.percentageOfRounded(0, 50));
356+
}
357+
358+
@Test
359+
public void percentageOfRoundedWithNegativeValues() {
360+
assertEquals(0, Tools.percentageOfRounded(-100, 50));
361+
assertEquals(0, Tools.percentageOfRounded(100, -50));
362+
assertEquals(0, Tools.percentageOfRounded(-100, -50));
363+
}
364+
365+
@Test
366+
public void percentageOfRoundedWhenValueExceedsTotal() {
367+
assertEquals(100, Tools.percentageOfRounded(100, 150));
368+
assertEquals(100, Tools.percentageOfRounded(50, 100));
369+
}
370+
371+
@Test
372+
public void percentageOfRoundedWithLargeValues() {
373+
// Test with large long values to ensure no overflow
374+
assertEquals(50, Tools.percentageOfRounded(Long.MAX_VALUE, Long.MAX_VALUE / 2));
375+
assertEquals(75, Tools.percentageOfRounded(1000000000000L, 750000000000L));
376+
}
377+
378+
@Test
379+
public void percentageOfRoundedRoundingBehavior() {
380+
// Test rounding behavior (Math.round)
381+
assertEquals(33, Tools.percentageOfRounded(3, 1)); // 33.33... -> 33
382+
assertEquals(67, Tools.percentageOfRounded(3, 2)); // 66.66... -> 67
383+
assertEquals(100, Tools.percentageOfRounded(1000, 999)); // 99.9 -> 100
384+
}
385+
386+
@Test
387+
public void percentageOfNormalCases() {
388+
assertEquals(0.0, Tools.percentageOf(100, 0), 0.0);
389+
assertEquals(1.0, Tools.percentageOf(100, 1), 0.0);
390+
assertEquals(50.0, Tools.percentageOf(100, 50), 0.0);
391+
assertEquals(99.0, Tools.percentageOf(100, 99), 0.0);
392+
assertEquals(100.0, Tools.percentageOf(100, 100), 0.0);
393+
assertEquals(25.0, Tools.percentageOf(200, 50), 0.0);
394+
assertEquals(33.333333333333336, Tools.percentageOf(300, 100), 0.000001);
395+
}
396+
397+
@Test
398+
public void percentageOfWithZeroTotal() {
399+
assertEquals(0.0, Tools.percentageOf(0, 0), 0.0);
400+
assertEquals(0.0, Tools.percentageOf(0, 50), 0.0);
401+
}
402+
403+
@Test
404+
public void percentageOfWithNegativeValues() {
405+
assertEquals(0.0, Tools.percentageOf(-100, 50), 0.0);
406+
assertEquals(0.0, Tools.percentageOf(100, -50), 0.0);
407+
assertEquals(0.0, Tools.percentageOf(-100, -50), 0.0);
408+
}
409+
410+
@Test
411+
public void percentageOfWhenValueExceedsTotal() {
412+
assertEquals(100.0, Tools.percentageOf(100, 150), 0.0);
413+
assertEquals(100.0, Tools.percentageOf(50, 100), 0.0);
414+
}
415+
416+
@Test
417+
public void percentageOfWithLargeValues() {
418+
// Test with large long values to ensure no overflow
419+
assertEquals(50.0, Tools.percentageOf(Long.MAX_VALUE, Long.MAX_VALUE / 2), 0.1);
420+
assertEquals(75.0, Tools.percentageOf(1000000000000L, 750000000000L), 0.0);
421+
}
422+
423+
@Test
424+
public void percentageOfPrecision() {
425+
// Test that double precision is maintained
426+
assertEquals(33.333333333333336, Tools.percentageOf(3, 1), 0.000001);
427+
assertEquals(66.66666666666667, Tools.percentageOf(3, 2), 0.000001);
428+
assertEquals(99.9, Tools.percentageOf(1000, 999), 0.0);
429+
assertEquals(12.5, Tools.percentageOf(8, 1), 0.0);
430+
assertEquals(0.1, Tools.percentageOf(1000, 1), 0.0);
431+
}
432+
433+
@Test
434+
public void percentageOfComparedToRounded() {
435+
// Verify that percentageOf returns precise values while percentageOfRounded rounds
436+
double precise = Tools.percentageOf(3, 1);
437+
int rounded = Tools.percentageOfRounded(3, 1);
438+
assertThat(precise).isEqualTo(33.333333333333336);
439+
assertThat(rounded).isEqualTo(33);
440+
441+
precise = Tools.percentageOf(3, 2);
442+
rounded = Tools.percentageOfRounded(3, 2);
443+
assertThat(precise).isEqualTo(66.66666666666667);
444+
assertThat(rounded).isEqualTo(67);
445+
446+
// Test edge case near 0.5 boundary
447+
precise = Tools.percentageOf(1000, 995);
448+
rounded = Tools.percentageOfRounded(1000, 995);
449+
assertThat(precise).isEqualTo(99.5);
450+
assertThat(rounded).isEqualTo(100);
451+
}
340452
}

0 commit comments

Comments
 (0)