Skip to content

Commit 7b61793

Browse files
authored
Fix compaction mods metrics update (#17636)
1 parent 9793441 commit 7b61793

3 files changed

Lines changed: 112 additions & 8 deletions

File tree

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/modification/ModificationFile.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.io.OutputStream;
4141
import java.nio.channels.FileChannel;
4242
import java.nio.file.Files;
43+
import java.nio.file.StandardCopyOption;
4344
import java.util.ArrayList;
4445
import java.util.Collection;
4546
import java.util.Collections;
@@ -334,10 +335,11 @@ public boolean exists() {
334335
public void remove() throws IOException {
335336
lock.writeLock().lock();
336337
try {
338+
long sizeBeforeRemove = fileExists ? getFileLength() : 0;
337339
close();
338340
FileUtils.deleteFileOrDirectory(file);
339341
if (fileExists) {
340-
updateModFileMetric(-1, -getFileLength());
342+
updateModFileMetric(-1, -sizeBeforeRemove);
341343
}
342344
fileExists = false;
343345
removed = true;
@@ -394,12 +396,12 @@ public String toString() {
394396
public void compact() throws IOException {
395397
long originFileSize = getFileLength();
396398
if (originFileSize > COMPACT_THRESHOLD && !hasCompacted) {
399+
File compactedFile = new File(getFile().getPath() + COMPACT_SUFFIX);
397400
try {
398401
Map<PartialPath, List<ModEntry>> pathModificationMap =
399402
getAllMods().stream().collect(Collectors.groupingBy(ModEntry::keyOfPatternTree));
400-
String newModsFileName = getFile().getPath() + COMPACT_SUFFIX;
401403
try (ModificationFile compactedModificationFile =
402-
new ModificationFile(newModsFileName, false)) {
404+
new ModificationFile(compactedFile, false)) {
403405
Set<Entry<PartialPath, List<ModEntry>>> modificationsEntrySet =
404406
pathModificationMap.entrySet();
405407
for (Map.Entry<PartialPath, List<ModEntry>> modificationEntry : modificationsEntrySet) {
@@ -408,12 +410,15 @@ public void compact() throws IOException {
408410
}
409411
} catch (IOException e) {
410412
LOGGER.error("compact mods file exception of {}", file, e);
413+
throw e;
411414
}
412-
// remove origin mods file
413-
this.remove();
414-
fileExists = true;
415-
// rename new mods file to origin name
416-
Files.move(new File(newModsFileName).toPath(), file.toPath());
415+
long compactedFileSize = compactedFile.length();
416+
close();
417+
Files.move(compactedFile.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING);
418+
if (updateMetrics) {
419+
FileMetrics.getInstance().increaseModFileSize(compactedFileSize - originFileSize);
420+
}
421+
fileExists = compactedFileSize > 0;
417422
LOGGER.info("{} settle successful", file);
418423

419424
if (getFileLength() > COMPACT_THRESHOLD) {
@@ -424,6 +429,7 @@ public void compact() throws IOException {
424429
}
425430
} catch (IOException e) {
426431
LOGGER.error("remove origin file or rename new mods file error.", e);
432+
throw e;
427433
}
428434
hasCompacted = true;
429435
}

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/dataregion/compaction/utils/CompactionUtilsTest.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,15 @@
2020
package org.apache.iotdb.db.storageengine.dataregion.compaction.utils;
2121

2222
import org.apache.iotdb.commons.exception.MetadataException;
23+
import org.apache.iotdb.commons.path.MeasurementPath;
2324
import org.apache.iotdb.commons.path.PartialPath;
2425
import org.apache.iotdb.db.exception.StorageEngineException;
26+
import org.apache.iotdb.db.service.metrics.FileMetrics;
2527
import org.apache.iotdb.db.storageengine.dataregion.compaction.AbstractCompactionTest;
2628
import org.apache.iotdb.db.storageengine.dataregion.compaction.execute.utils.CompactionPathUtils;
29+
import org.apache.iotdb.db.storageengine.dataregion.compaction.execute.utils.CompactionUtils;
30+
import org.apache.iotdb.db.storageengine.dataregion.modification.ModificationFile;
31+
import org.apache.iotdb.db.storageengine.dataregion.modification.TreeDeletionEntry;
2732

2833
import org.apache.tsfile.exception.write.WriteProcessException;
2934
import org.apache.tsfile.file.metadata.IDeviceID;
@@ -32,6 +37,7 @@
3237
import org.junit.Test;
3338

3439
import java.io.IOException;
40+
import java.util.ArrayList;
3541

3642
public class CompactionUtilsTest extends AbstractCompactionTest {
3743
@Override
@@ -54,4 +60,34 @@ public void testCompactionPathUtils() {
5460
Assert.fail();
5561
}
5662
}
63+
64+
@Test
65+
public void testDeleteSourceTsFileUpdatesModMetrics() throws Exception {
66+
int modFileNumBefore = FileMetrics.getInstance().getModFileNum();
67+
long modFileSizeBefore = FileMetrics.getInstance().getModFileSize();
68+
69+
createFiles(2, 1, 1, 10, 0, 0, 10, 10, false, true);
70+
71+
long totalModFileSize = 0;
72+
for (int i = 0; i < seqResources.size(); i++) {
73+
try (ModificationFile modificationFile = seqResources.get(i).getModFileForWrite()) {
74+
modificationFile.write(
75+
new TreeDeletionEntry(
76+
new MeasurementPath(new String[] {COMPACTION_TEST_SG, "d0", "s0"}),
77+
Long.MIN_VALUE,
78+
i + 10));
79+
totalModFileSize += modificationFile.getFileLength();
80+
}
81+
}
82+
83+
Assert.assertEquals(
84+
modFileNumBefore + seqResources.size(), FileMetrics.getInstance().getModFileNum());
85+
Assert.assertEquals(
86+
modFileSizeBefore + totalModFileSize, FileMetrics.getInstance().getModFileSize());
87+
88+
CompactionUtils.deleteSourceTsFileAndUpdateFileMetrics(new ArrayList<>(seqResources), true);
89+
90+
Assert.assertEquals(modFileNumBefore, FileMetrics.getInstance().getModFileNum());
91+
Assert.assertEquals(modFileSizeBefore, FileMetrics.getInstance().getModFileSize());
92+
}
5793
}

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/dataregion/modification/ModificationFileTest.java

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.apache.iotdb.commons.exception.IllegalPathException;
2323
import org.apache.iotdb.commons.path.MeasurementPath;
24+
import org.apache.iotdb.db.service.metrics.FileMetrics;
2425
import org.apache.iotdb.db.storageengine.dataregion.compaction.execute.recover.CompactionRecoverManager;
2526
import org.apache.iotdb.db.storageengine.dataregion.modification.IDPredicate.FullExactMatch;
2627
import org.apache.iotdb.db.storageengine.dataregion.modification.IDPredicate.NOP;
@@ -50,6 +51,67 @@
5051

5152
public class ModificationFileTest {
5253

54+
@Test
55+
public void testRemoveUpdatesMetrics() throws IOException {
56+
String tempFileName = TestConstant.BASE_OUTPUT_PATH.concat("mod.remove.metrics.temp");
57+
int modFileNumBefore = FileMetrics.getInstance().getModFileNum();
58+
long modFileSizeBefore = FileMetrics.getInstance().getModFileSize();
59+
try (ModificationFile modificationFile = new ModificationFile(tempFileName, true)) {
60+
modificationFile.write(
61+
new TreeDeletionEntry(
62+
new MeasurementPath(new String[] {"root", "sg", "d1", "s1"}), 1, 10));
63+
long fileLength = modificationFile.getFileLength();
64+
assertEquals(modFileNumBefore + 1, FileMetrics.getInstance().getModFileNum());
65+
assertEquals(modFileSizeBefore + fileLength, FileMetrics.getInstance().getModFileSize());
66+
67+
modificationFile.remove();
68+
assertEquals(modFileNumBefore, FileMetrics.getInstance().getModFileNum());
69+
assertEquals(modFileSizeBefore, FileMetrics.getInstance().getModFileSize());
70+
} finally {
71+
Files.deleteIfExists(new File(tempFileName).toPath());
72+
}
73+
}
74+
75+
@Test
76+
public void testCompactUpdatesMetricsAndAllowFurtherWrite() throws IOException {
77+
String tempFileName = TestConstant.BASE_OUTPUT_PATH.concat("mod.compact.metrics.temp");
78+
int modFileNumBefore = FileMetrics.getInstance().getModFileNum();
79+
long modFileSizeBefore = FileMetrics.getInstance().getModFileSize();
80+
long time = 1000;
81+
try (ModificationFile modificationFile = new ModificationFile(tempFileName, true)) {
82+
while (modificationFile.getFileLength() < 1024 * 1024) {
83+
modificationFile.write(
84+
new TreeDeletionEntry(
85+
new MeasurementPath(new String[] {"root", "sg", "d1", "s1"}),
86+
Long.MIN_VALUE,
87+
time += 5000));
88+
}
89+
90+
assertEquals(modFileNumBefore + 1, FileMetrics.getInstance().getModFileNum());
91+
modificationFile.compact();
92+
assertEquals(modFileNumBefore + 1, FileMetrics.getInstance().getModFileNum());
93+
assertEquals(
94+
modFileSizeBefore + modificationFile.getFileLength(),
95+
FileMetrics.getInstance().getModFileSize());
96+
97+
modificationFile.write(
98+
new TreeDeletionEntry(
99+
new MeasurementPath(new String[] {"root", "sg", "d1", "s2"}),
100+
Long.MIN_VALUE,
101+
time + 5000));
102+
assertEquals(modFileNumBefore + 1, FileMetrics.getInstance().getModFileNum());
103+
assertEquals(
104+
modFileSizeBefore + modificationFile.getFileLength(),
105+
FileMetrics.getInstance().getModFileSize());
106+
107+
modificationFile.remove();
108+
assertEquals(modFileNumBefore, FileMetrics.getInstance().getModFileNum());
109+
assertEquals(modFileSizeBefore, FileMetrics.getInstance().getModFileSize());
110+
} finally {
111+
Files.deleteIfExists(new File(tempFileName).toPath());
112+
}
113+
}
114+
53115
@Test
54116
public void readMyWrite() {
55117
String tempFileName = TestConstant.BASE_OUTPUT_PATH.concat("mod.temp");

0 commit comments

Comments
 (0)