Skip to content

Commit cafd9dd

Browse files
adoroszlaiadoroszlai
authored
HDDS-14176. Do not use Optional for volumeUsage (#9503)
1 parent 33544de commit cafd9dd

11 files changed

Lines changed: 75 additions & 73 deletions

File tree

‎hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/HddsVolume.java‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public class HddsVolume extends StorageVolume {
107107
// and stored as a member to prevent spawning lots of File objects.
108108
private File dbParentDir;
109109
private File deletedContainerDir;
110-
private AtomicBoolean dbLoaded = new AtomicBoolean(false);
110+
private final AtomicBoolean dbLoaded = new AtomicBoolean(false);
111111
private final AtomicBoolean dbLoadFailure = new AtomicBoolean(false);
112112

113113
private final int volumeTestCount;
@@ -328,7 +328,7 @@ public VolumeCheckResult checkDbHealth(File dbFile) throws InterruptedException
328328

329329
final boolean isVolumeTestResultHealthy = true;
330330
try (ManagedOptions managedOptions = new ManagedOptions();
331-
ManagedRocksDB readOnlyDb = ManagedRocksDB.openReadOnly(managedOptions, dbFile.toString())) {
331+
ManagedRocksDB ignored = ManagedRocksDB.openReadOnly(managedOptions, dbFile.toString())) {
332332
volumeTestResultQueue.add(isVolumeTestResultHealthy);
333333
} catch (Exception e) {
334334
if (Thread.currentThread().isInterrupted()) {
@@ -357,28 +357,35 @@ public VolumeCheckResult checkDbHealth(File dbFile) throws InterruptedException
357357
return VolumeCheckResult.HEALTHY;
358358
}
359359

360-
@VisibleForTesting
361-
public void checkVolumeUsages() {
360+
void checkVolumeUsages() {
362361
boolean isEnoughSpaceAvailable = true;
363362
SpaceUsageSource currentUsage = getCurrentUsage();
364363
long getFreeSpaceToSpare = getFreeSpaceToSpare(currentUsage.getCapacity());
365-
if (currentUsage.getAvailable() < getFreeSpaceToSpare) {
364+
final long committed = committedBytes.get();
365+
final long available = currentUsage.getAvailable();
366+
if (available < getFreeSpaceToSpare) {
366367
LOG.warn("Volume {} has insufficient space for write operation. Available: {}, Free space to spare: {}",
367-
getStorageDir(), currentUsage.getAvailable(), getFreeSpaceToSpare);
368+
getStorageDir(), available, getFreeSpaceToSpare);
368369
isEnoughSpaceAvailable = false;
369-
} else if (committedBytes.get() > 0 && currentUsage.getAvailable() < committedBytes.get() + getFreeSpaceToSpare) {
370+
} else if (committed > 0 && available < committed + getFreeSpaceToSpare) {
370371
LOG.warn("Volume {} has insufficient space for on-going container write operation. " +
371372
"Committed: {}, Available: {}, Free space to spare: {}",
372-
getStorageDir(), committedBytes.get(), currentUsage.getAvailable(), getFreeSpaceToSpare);
373+
getStorageDir(), committed, available, getFreeSpaceToSpare);
373374
isEnoughSpaceAvailable = false;
374375
}
375376

376377
volumeInfoMetrics.setAvailableSpaceInsufficient(!isEnoughSpaceAvailable);
377378

378-
if (!getVolumeUsage().map(VolumeUsage::isReservedUsagesInRange).orElse(true)) {
379-
LOG.warn("Volume {} reserved usages is higher than actual allocated reserved space.",
380-
getStorageDir());
381-
volumeInfoMetrics.setReservedCrossesLimit(true);
379+
final VolumeUsage usage = getVolumeUsage();
380+
if (usage != null && usage.getReservedInBytes() > 0) {
381+
final SpaceUsageSource realUsage = usage.realUsage();
382+
long reservedUsed = VolumeUsage.getOtherUsed(realUsage);
383+
final boolean crossesLimit = reservedUsed > usage.getReservedInBytes();
384+
if (crossesLimit) {
385+
LOG.warn("Volume {} reserved usages {} is higher than actual allocated reserved space {}. (Real usage: {})",
386+
getStorageDir(), reservedUsed, usage.getReservedInBytes(), realUsage);
387+
}
388+
volumeInfoMetrics.setReservedCrossesLimit(crossesLimit);
382389
} else {
383390
volumeInfoMetrics.setReservedCrossesLimit(false);
384391
}
@@ -552,7 +559,6 @@ public long getContainerCount() {
552559
/**
553560
* Pick a DbVolume for HddsVolume and init db instance.
554561
* Use the HddsVolume directly if no DbVolume found.
555-
* @param dbVolumeSet
556562
*/
557563
public void createDbStore(MutableVolumeSet dbVolumeSet) throws IOException {
558564
DbVolume chosenDbVolume = null;

‎hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.nio.file.Path;
2929
import java.util.LinkedList;
3030
import java.util.Objects;
31-
import java.util.Optional;
3231
import java.util.Properties;
3332
import java.util.Queue;
3433
import java.util.UUID;
@@ -99,7 +98,7 @@ public abstract class StorageVolume implements Checkable<Boolean, VolumeCheckRes
9998
private File tmpDir;
10099
private File diskCheckDir;
101100

102-
private final Optional<VolumeUsage> volumeUsage;
101+
private final VolumeUsage volumeUsage;
103102

104103
private final VolumeSet volumeSet;
105104

@@ -152,7 +151,7 @@ protected StorageVolume(Builder<?> b) throws IOException {
152151
storageDir = new File(location.getUri().getPath(), b.storageDirStr);
153152
SpaceUsageCheckParams checkParams = getSpaceUsageCheckParams(b, this::getContainerDirsPath);
154153
checkParams.setContainerUsedSpace(this::containerUsedSpace);
155-
volumeUsage = Optional.of(new VolumeUsage(checkParams, b.conf));
154+
volumeUsage = new VolumeUsage(checkParams, b.conf);
156155
this.volumeSet = b.volumeSet;
157156
this.state = VolumeState.NOT_INITIALIZED;
158157
this.clusterID = b.clusterID;
@@ -167,7 +166,7 @@ protected StorageVolume(Builder<?> b) throws IOException {
167166
this.healthCheckFileSize = dnConf.getVolumeHealthCheckFileSize();
168167
} else {
169168
storageDir = new File(b.volumeRootStr);
170-
volumeUsage = Optional.empty();
169+
volumeUsage = null;
171170
this.volumeSet = null;
172171
this.storageID = UUID.randomUUID().toString();
173172
this.state = VolumeState.FAILED;
@@ -199,7 +198,9 @@ public void format(String cid) throws IOException {
199198
}
200199

201200
public void start() throws IOException {
202-
volumeUsage.ifPresent(VolumeUsage::start);
201+
if (volumeUsage != null) {
202+
volumeUsage.start();
203+
}
203204
}
204205

205206
/**
@@ -468,8 +469,7 @@ public String getVolumeRootDir() {
468469

469470
/** Get current usage of the volume. */
470471
public SpaceUsageSource getCurrentUsage() {
471-
return volumeUsage.map(VolumeUsage::getCurrentUsage)
472-
.orElse(SpaceUsageSource.UNKNOWN);
472+
return volumeUsage != null ? volumeUsage.getCurrentUsage() : SpaceUsageSource.UNKNOWN;
473473
}
474474

475475
protected StorageLocationReport.Builder reportBuilder() {
@@ -480,11 +480,11 @@ protected StorageLocationReport.Builder reportBuilder() {
480480
.setStorageType(storageType);
481481

482482
if (!builder.isFailed()) {
483-
SpaceUsageSource usage = getCurrentUsage();
483+
SpaceUsageSource usage = volumeUsage.getCurrentUsage();
484484
builder.setCapacity(usage.getCapacity())
485485
.setRemaining(usage.getAvailable())
486-
.setScmUsed(usage.getUsedSpace());
487-
getVolumeUsage().ifPresent(vu -> builder.setReserved(vu.getReservedInBytes()));
486+
.setScmUsed(usage.getUsedSpace())
487+
.setReserved(volumeUsage.getReservedInBytes());
488488
}
489489

490490
return builder;
@@ -512,22 +512,26 @@ public File getDiskCheckDir() {
512512
}
513513

514514
public void refreshVolumeUsage() {
515-
volumeUsage.ifPresent(VolumeUsage::refreshNow);
515+
if (volumeUsage != null) {
516+
volumeUsage.refreshNow();
517+
}
516518
}
517519

518520
/** @see #getCurrentUsage() */
519-
public Optional<VolumeUsage> getVolumeUsage() {
521+
public VolumeUsage getVolumeUsage() {
520522
return volumeUsage;
521523
}
522524

523525
public void incrementUsedSpace(long usedSpace) {
524-
volumeUsage.ifPresent(usage -> usage
525-
.incrementUsedSpace(usedSpace));
526+
if (volumeUsage != null) {
527+
volumeUsage.incrementUsedSpace(usedSpace);
528+
}
526529
}
527530

528531
public void decrementUsedSpace(long reclaimedSpace) {
529-
volumeUsage.ifPresent(usage -> usage
530-
.decrementUsedSpace(reclaimedSpace));
532+
if (volumeUsage != null) {
533+
volumeUsage.decrementUsedSpace(reclaimedSpace);
534+
}
531535
}
532536

533537
public VolumeSet getVolumeSet() {
@@ -580,12 +584,16 @@ public DatanodeConfiguration getDatanodeConfig() {
580584

581585
public void failVolume() {
582586
setState(VolumeState.FAILED);
583-
volumeUsage.ifPresent(VolumeUsage::shutdown);
587+
if (volumeUsage != null) {
588+
volumeUsage.shutdown();
589+
}
584590
}
585591

586592
public void shutdown() {
587593
setState(VolumeState.NON_EXISTENT);
588-
volumeUsage.ifPresent(VolumeUsage::shutdown);
594+
if (volumeUsage != null) {
595+
volumeUsage.shutdown();
596+
}
589597
cleanTmpDiskCheckDir();
590598
}
591599

‎hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeInfoMetrics.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ public void incNumScansSkipped() {
183183
public void getMetrics(MetricsCollector collector, boolean all) {
184184
MetricsRecordBuilder builder = collector.addRecord(metricsSourceName);
185185
registry.snapshot(builder, all);
186-
volume.getVolumeUsage().ifPresent(volumeUsage -> {
186+
VolumeUsage volumeUsage = volume.getVolumeUsage();
187+
if (volumeUsage != null) {
187188
SpaceUsageSource usage = volumeUsage.getCurrentUsage();
188189
long reserved = volumeUsage.getReservedInBytes();
189190
builder
@@ -192,6 +193,6 @@ public void getMetrics(MetricsCollector collector, boolean all) {
192193
.addGauge(USED, usage.getUsedSpace())
193194
.addGauge(RESERVED, reserved)
194195
.addGauge(TOTAL_CAPACITY, usage.getCapacity() + reserved);
195-
});
196+
}
196197
}
197198
}

‎hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED_PERCENT;
2222
import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT;
2323

24-
import com.google.common.annotations.VisibleForTesting;
2524
import java.io.File;
2625
import java.io.IOException;
2726
import java.util.Collection;
@@ -108,7 +107,6 @@ public class VolumeUsage {
108107
Preconditions.assertTrue(reservedInBytes >= 0, reservedInBytes + " < 0");
109108
}
110109

111-
@VisibleForTesting
112110
SpaceUsageSource realUsage() {
113111
return source.snapshot();
114112
}
@@ -150,7 +148,7 @@ public void decrementUsedSpace(long reclaimedSpace) {
150148
* so there could be that DU value > totalUsed when there are deletes.
151149
* @return other used space
152150
*/
153-
private static long getOtherUsed(SpaceUsageSource usage) {
151+
static long getOtherUsed(SpaceUsageSource usage) {
154152
long totalUsed = usage.getCapacity() - usage.getAvailable();
155153
return Math.max(totalUsed - usage.getUsedSpace(), 0L);
156154
}
@@ -237,14 +235,4 @@ private static long getReserved(ConfigurationSource conf, String rootDir,
237235

238236
return (long) Math.ceil(capacity * percentage);
239237
}
240-
241-
public boolean isReservedUsagesInRange() {
242-
SpaceUsageSource spaceUsageSource = realUsage();
243-
long reservedUsed = getOtherUsed(spaceUsageSource);
244-
if (reservedInBytes > 0 && reservedUsed > reservedInBytes) {
245-
LOG.warn("Reserved usages {} is higher than actual allocated reserved space {}.", reservedUsed, reservedInBytes);
246-
return false;
247-
}
248-
return true;
249-
}
250238
}

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,7 @@ public void testContainerCloseActionWhenVolumeFull(
339339
HddsDispatcher hddsDispatcher = new HddsDispatcher(
340340
conf, containerSet, volumeSet, handlers, context, metrics, null);
341341
hddsDispatcher.setClusterId(scmId.toString());
342-
containerData.getVolume().getVolumeUsage()
343-
.ifPresent(usage -> usage.incrementUsedSpace(60));
342+
containerData.getVolume().incrementUsedSpace(60);
344343
usedSpace.addAndGet(60);
345344
ContainerCommandResponseProto response = hddsDispatcher
346345
.dispatch(getWriteChunkRequest(dd.getUuidString(), 1L, 1L), null);

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestReservedVolumeSpace.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public void testDefaultConfig() throws Exception {
6868
// Gets the total capacity reported by Ozone, which may be limited to less than the volume's real capacity by the
6969
// DU reserved configurations.
7070
long volumeCapacity = hddsVolume.getCurrentUsage().getCapacity();
71-
VolumeUsage usage = hddsVolume.getVolumeUsage().get();
71+
VolumeUsage usage = hddsVolume.getVolumeUsage();
7272

7373
// Gets the actual total capacity without accounting for DU reserved space configurations.
7474
long totalCapacity = usage.realUsage().getCapacity();
@@ -92,7 +92,7 @@ public void testVolumeCapacityAfterReserve() throws Exception {
9292
HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT);
9393

9494
long volumeCapacity = hddsVolume.getCurrentUsage().getCapacity();
95-
VolumeUsage usage = hddsVolume.getVolumeUsage().get();
95+
VolumeUsage usage = hddsVolume.getVolumeUsage();
9696

9797
//Gets the actual total capacity
9898
long totalCapacity = usage.realUsage().getCapacity();
@@ -116,8 +116,7 @@ public void testReservedWhenBothConfigSet() throws Exception {
116116
folder.toString() + ":500B");
117117
HddsVolume hddsVolume = volumeBuilder.conf(conf).build();
118118

119-
long reservedFromVolume = hddsVolume.getVolumeUsage().get()
120-
.getReservedInBytes();
119+
long reservedFromVolume = hddsVolume.getVolumeUsage().getReservedInBytes();
121120
assertEquals(500, reservedFromVolume);
122121
}
123122

@@ -130,7 +129,7 @@ public void testFallbackToPercentConfig() throws Exception {
130129
temp.toString() + ":500B");
131130
HddsVolume hddsVolume = volumeBuilder.conf(conf).build();
132131

133-
VolumeUsage usage = hddsVolume.getVolumeUsage().get();
132+
VolumeUsage usage = hddsVolume.getVolumeUsage();
134133
long reservedFromVolume = usage.getReservedInBytes();
135134
assertNotEquals(0, reservedFromVolume);
136135

@@ -150,8 +149,7 @@ public void testInvalidConfig() throws Exception {
150149
folder.toString() + ":500C");
151150
HddsVolume hddsVolume1 = volumeBuilder.conf(conf1).build();
152151

153-
long reservedFromVolume1 = hddsVolume1.getVolumeUsage().get()
154-
.getReservedInBytes();
152+
long reservedFromVolume1 = hddsVolume1.getVolumeUsage().getReservedInBytes();
155153
assertEquals(getExpectedDefaultReserved(hddsVolume1), reservedFromVolume1);
156154

157155
OzoneConfiguration conf2 = new OzoneConfiguration();
@@ -160,8 +158,7 @@ public void testInvalidConfig() throws Exception {
160158
conf2.set(HDDS_DATANODE_DIR_DU_RESERVED_PERCENT, "20");
161159
HddsVolume hddsVolume2 = volumeBuilder.conf(conf2).build();
162160

163-
long reservedFromVolume2 = hddsVolume2.getVolumeUsage().get()
164-
.getReservedInBytes();
161+
long reservedFromVolume2 = hddsVolume2.getVolumeUsage().getReservedInBytes();
165162
assertEquals(getExpectedDefaultReserved(hddsVolume2), reservedFromVolume2);
166163
}
167164

@@ -187,7 +184,7 @@ public void testPathsCanonicalized() throws Exception {
187184
conf.set(ScmConfigKeys.HDDS_DATANODE_DIR_DU_RESERVED, symlink + ":500B");
188185
HddsVolume hddsVolume = volumeBuilder.conf(conf).build();
189186

190-
long reservedFromVolume = hddsVolume.getVolumeUsage().get().getReservedInBytes();
187+
long reservedFromVolume = hddsVolume.getVolumeUsage().getReservedInBytes();
191188
assertEquals(500, reservedFromVolume);
192189
}
193190

@@ -210,7 +207,7 @@ public void testMinFreeSpaceCalculator() throws Exception {
210207
}
211208

212209
private long getExpectedDefaultReserved(HddsVolume volume) {
213-
long totalCapacity = volume.getVolumeUsage().get().realUsage().getCapacity();
210+
long totalCapacity = volume.getVolumeUsage().realUsage().getCapacity();
214211
return (long) Math.ceil(totalCapacity * HDDS_DATANODE_DIR_DU_RESERVED_PERCENT_DEFAULT);
215212
}
216213
}

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestStorageVolumeHealthChecks.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public void verifyFullVolumeHealthWithDiskReadWriteStatus(StorageVolume.Builder<
122122
for (boolean result : checkResult) {
123123
StorageVolume volume = builder.build();
124124

125-
VolumeUsage usage = volume.getVolumeUsage().get();
125+
VolumeUsage usage = volume.getVolumeUsage();
126126
DatanodeConfiguration dnConf = CONF.getObject(DatanodeConfiguration.class);
127127
int minimumDiskSpace = dnConf.getVolumeHealthCheckFileSize() * 2;
128128
// Keep remaining space as just less than double of VolumeHealthCheckFileSize.

‎hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ public void testShutdown() throws Exception {
235235

236236
// Verify that volume usage can be queried during shutdown.
237237
for (StorageVolume volume : volumesList) {
238-
assertThat(volume.getVolumeUsage()).isPresent();
238+
assertThat(volume.getVolumeUsage()).isNotNull();
239239
volume.getCurrentUsage();
240240
}
241241
}

‎hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestMiniOzoneCluster.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ public void testMultipleDataDirs() throws Exception {
276276

277277
volumeList.forEach(storageVolume -> assertEquals(
278278
(long) StorageSize.parse(reservedSpace).getValue(),
279-
storageVolume.getVolumeUsage().get().getReservedInBytes()));
279+
storageVolume.getVolumeUsage().getReservedInBytes()));
280280
}
281281

282282
}

0 commit comments

Comments
 (0)