Support WAL Compression #12476

THUMarkLau · 2024-05-07T08:11:42Z

The write-ahead logging consumes a significant amount of I/O resources during data writing, sometimes even 2-7 times the amount of TsFile I/O. Currently, the write-ahead logging in the system is uncompressed, this is a main reason for the large volume of WAL. Therefore, compressing the write-ahead logging can save I/O resources and enhance system performance. This PR primarily introduces support for compressing write-ahead logging, featuring the following characteristics:

The basic unit for compression and decompression is a WAL Buffer, which may contain several WAL Entries, with a default size of 16MB. This design achieves a good compression ratio while minimizing modifications to upper-layer applications.
To maintain compatibility with previous versions, we have replaced the original Magic String in the WAL file with a Version String to distinguish between different versions of WAL. If there are subsequent new versions of WAL, we can continue to add new Version Strings.
Multiple compression methods are supported. Currently, the choice of compression method is not open to users, but theoretically, our design supports various compression methods, and we may open this option in the future.

Changes in system resource consumption and system performance caused by this feature will be supplemented in subsequent comments.

codecov · 2024-05-25T12:28:53Z

Codecov Report

Attention: Patch coverage is 83.50515% with 48 lines in your changes missing coverage. Please review.

Project coverage is 42.16%. Comparing base (a4a7be0) to head (b99c977).
Report is 64 commits behind head on master.

❗ Current head b99c977 differs from pull request most recent head 7201ed6

Please upload reports for the commit 7201ed6 to get more accurate results.

Files	Patch %	Lines
...torageengine/dataregion/wal/io/WALInputStream.java	81.25%	33 Missing ⚠️
...eengine/dataregion/wal/utils/WALEntryPosition.java	50.00%	7 Missing ⚠️
...java/org/apache/iotdb/db/conf/IoTDBDescriptor.java	33.33%	4 Missing ⚠️
...wal/allocation/AbstractNodeAllocationStrategy.java	0.00%	3 Missing ⚠️
...b/storageengine/dataregion/wal/io/WALMetaData.java	88.88%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #12476      +/-   ##
============================================
- Coverage     43.06%   42.16%   -0.91%     
- Complexity       69       71       +2     
============================================
  Files          3202     3262      +60     
  Lines        200013   205915    +5902     
  Branches      23853    24556     +703     
============================================
+ Hits          86139    86814     +675     
- Misses       113874   119101    +5227

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Caideyipi · 2024-05-28T15:16:25Z

...re/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/io/LogWriter.java

@@ -79,6 +132,10 @@ public long size() {
    return size;
  }

+  public long oiginalSize() {


May be replaced with originalSize

Caideyipi · 2024-05-28T15:31:47Z

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBConfig.java

@@ -221,7 +222,7 @@ public class IoTDBConfig {
  private int walBufferQueueCapacity = 500;

  /** Size threshold of each wal file. Unit: byte */
-  private volatile long walFileSizeThresholdInByte = 30 * 1024 * 1024L;
+  private volatile long walFileSizeThresholdInByte = 30 * 1024L * 1024L;


Why change this here?

Caideyipi · 2024-05-28T15:32:16Z

iotdb-core/node-commons/src/assembly/resources/conf/iotdb-common.properties

@@ -763,6 +763,10 @@ data_replication_factor=1
 # Datatype: long
 # iot_consensus_cache_window_time_in_ms=-1

+# WAL compression algorithm
+# options: UNCOMPRESSED, SNAPPY, LZ4, GZIP, ZSTD
+# wal_compress_algorithm=UNCOMPRESSED


May tune to decide the default algorithm.

Change the configuration item to enable or disable wal compression by true or false.

remove metrics in mem table flush task, cache hash code in partial path, use gzip to compress wal batch update metrics

…shold

jt2594838

There are issues that should be solved. Also, please add new tests.

.../apache/iotdb/db/storageengine/dataregion/wal/allocation/AbstractNodeAllocationStrategy.java

...atanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/buffer/WALBuffer.java

jt2594838 · 2024-05-31T04:16:19Z

...atanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/buffer/WALBuffer.java

-          || (forceFlag && currentWALFileWriter.size() >= config.getWalFileSizeThresholdInByte())) {
+          || (forceFlag
+              && currentWALFileWriter.originalSize() >= config.getWalFileSizeThresholdInByte())) {


Using the originalSize to control force may result in small actual IOs if the compression ratio is high.
I guess this is to control the risk of losing persistence.
It would be better to add a comment explaining why you should choose originalSize over actualSize here.
Ideally, I would expect some experiments to show using originalSize is better than actualSize, or the trade-off.

It's @OneSizeFitsQuorum 's suggestion.
The main reason to control the writer with originalSize is that when flushing a WALEntry into a buffer, an instance of WALFlushListener will be created. These objects are released when the file writer is closed. If the compression ratio is high, and we are using the actual size to control the writer, a lot of WALFlushListener instance will be kept in memory and cause an OOM error. This happened in the unit-test, so we fix it in this way.

So, in the end, you want to control the number of WALFlushListeners (and ultimately their corresponding memory).
Then, why not just use the number of WALFlushListeners (or their corresponding memory) as one of the predicates?

The original WAL file open/close control used the size of the WAL file. This PR only made some simple modifications on top of that to accommodate compression. It is reasonable to control the opening and closing of WAL files using the number of WALFlushListeners, but this goes beyond the scope of this PR and would make the PR's integration cycle longer and the impact more unpredictable. Therefore, it is suggested that controlling the opening and closing of WAL files using WALFlushListeners be planned for future work.

Ok, then, please leave a TODO here.

...main/java/org/apache/iotdb/db/storageengine/dataregion/wal/checkpoint/CheckpointManager.java

.../datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/io/WALMetaData.java

jt2594838 · 2024-05-31T07:20:26Z

...e/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/utils/WALEntryPosition.java

+    try (WALInputStream is = openReadFileStream()) {
+      is.skipToGivenPosition(position);
      ByteBuffer buffer = ByteBuffer.allocate(size);
-      channel.position(position);
-      channel.read(buffer);
-      buffer.clear();
+      is.read(buffer);
+      buffer.flip();
      return buffer;
    }


I wonder about the efficiency of opening a stream for each WALEntry. Maybe the stream should be reused somehow.

My response is the same as in the previous comments: this is how the original design was implemented. I just made slight modifications to make it suitable for compressed cases. Theoretically, the performance is similar to the original since the original design involved opening a Channel, and a WALInputStream essentially operates as a Channel. Sharing the Stream is a reasonable suggestion, but it is unrelated to the current task (Write Ahead Log compression) and may introduce other bugs and performance issues, so I will not be modifying this part of the code.

Just leave a TODO here, then.

jt2594838 · 2024-05-31T07:22:46Z

...e/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/utils/WALEntryPosition.java

+    if (isInSealedFile()) {
+      walFile = walNode.getWALFile(walFileVersionId);
+      return new WALInputStream(walFile);
+    } else {
+      try {
+        walFile = walNode.getWALFile(walFileVersionId);
+        return new WALInputStream(walFile);
+      } catch (IOException e) {
+        // unsealed file may be renamed after sealed, so we should try again
+        if (isInSealedFile()) {
+          walFile = walNode.getWALFile(walFileVersionId);
+          return new WALInputStream(walFile);
+        } else {
+          throw e;
+        }
+      }
+    }


It seems the "else" branch is just enough, and the branch above is not significantly different from it.

Same as above, this is the original design.

Just leave a TODO here, then.

...st/java/org/apache/iotdb/db/storageengine/dataregion/wal/compression/WALCompressionTest.java

jt2594838 · 2024-06-04T01:48:52Z

...tanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/io/WALInputStream.java

          posRemain -= currSegmentSize;
+          channel.position(
+              channel.position()
+                  + currSegmentSize
+                  + (type == CompressionType.UNCOMPRESSED ? 0 : Integer.BYTES));


You may skip with the channel by currSegmentSize + Integer.BYTES if the WAL is compressed, but posRemain is only reduced by currSegmentSize.
Is this inconsistency problematic?

No. posRemain is a logical size, not a physical size. It represents the offset of the WAL before compression, so you only need to subtract the originalSegmentSize.

This should be clarified by comments or better variable names; try it.

OneSizeFitsQuorum · 2024-06-11T03:19:50Z

.../apache/iotdb/db/storageengine/dataregion/wal/allocation/AbstractNodeAllocationStrategy.java

@@ -85,6 +86,9 @@ protected IWALNode createWALNode(
    } catch (FileNotFoundException e) {
      logger.error("Fail to create wal node", e);
      return WALFakeNode.getFailureInstance(e);
+    } catch (IOException e) {


Exception handling should be the same as above

OneSizeFitsQuorum · 2024-06-11T04:02:30Z

iotdb-core/node-commons/src/assembly/resources/conf/iotdb-system.properties

@@ -1215,6 +1215,10 @@ data_replication_factor=1
 # Datatype: long
 # iot_consensus_cache_window_time_in_ms=-1

+# Enable Write Ahead Log compression.
+# Option: true, false
+# enable_wal_compression=false


enable hot reload?

sonarcloud · 2024-06-12T05:45:58Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

jt2594838

Good, please see to the remaining minor issues.

jt2594838 · 2024-06-12T10:21:57Z

...tanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/wal/io/WALInputStream.java

+      if (Objects.isNull(dataBuffer)
+          || dataBuffer.capacity() < segmentInfo.uncompressedSize
+          || dataBuffer.capacity() > segmentInfo.uncompressedSize * 2) {
+        dataBuffer = ByteBuffer.allocateDirect(segmentInfo.uncompressedSize);


If the previous buffer is not null, it should be manually released through MmapUtil.clean—the same for the below.

jt2594838 · 2024-06-12T10:34:59Z

...st/java/org/apache/iotdb/db/storageengine/dataregion/wal/compression/WALCompressionTest.java

+    // Do not compress it
+    IoTDBDescriptor.getInstance().getConfig().setWALCompressionAlgorithm(CompressionType.LZ4);


Do not compress it?

THUMarkLau force-pushed the wal-compress-formal-branch branch from 834e61d to 70d8400 Compare May 11, 2024 08:24

THUMarkLau force-pushed the wal-compress-formal-branch branch 2 times, most recently from 89b0f35 to 94152b2 Compare May 24, 2024 06:04

THUMarkLau force-pushed the wal-compress-formal-branch branch from e7250f8 to 069795d Compare May 26, 2024 06:25

Caideyipi reviewed May 28, 2024

View reviewed changes

THUMarkLau added 22 commits May 31, 2024 08:33

enable wal compression

0c17c07

remove metrics in mem table flush task, cache hash code in partial path, use gzip to compress wal batch update metrics

fix bug

58220e0

fix compilation problem

94e7c1d

remove useless code

15b44ad

recover some code

bb27b94

support compression type in WAL Compress Header

e09008d

support multi version WAL

39e3989

edit configuration item

208c16a

add log for WAL size

06dfd71

temp for debug

b046422

fix bug

eb5b5b2

remove useless log

cf0af96

remove one configuration

04979bd

use compression rate to update wal disk usage

5c0e656

fix ut

2624ba6

fix test

aa88a1b

set default to uncompress

4161155

fix wal ut

4d39a6f

optimize calculating of wal size

8c9a8e6

close wal file when the origin size of wal buffer is larger than thre…

fa2c12b

…shold

add the size of magic string

33b872c

may be fix the bug

93a6482

THUMarkLau force-pushed the wal-compress-formal-branch branch from eb167a8 to 93a6482 Compare May 31, 2024 00:35

jt2594838 requested changes May 31, 2024

View reviewed changes

fix with comment

a9b30dc

jt2594838 reviewed Jun 4, 2024

View reviewed changes

THUMarkLau added 2 commits June 9, 2024 16:00

edit with review

6e2dcb2

fix test

75afa65

OneSizeFitsQuorum reviewed Jun 11, 2024

View reviewed changes

THUMarkLau added 4 commits June 11, 2024 16:50

add test for wal compression

dd61857

add hot reload

19764fa

clean the code to make it more readable

b99c977

reuse the byte buffer if possible

7201ed6

jt2594838 approved these changes Jun 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support WAL Compression #12476

Support WAL Compression #12476

THUMarkLau commented May 7, 2024 •

edited

codecov bot commented May 25, 2024 •

edited

Caideyipi May 28, 2024

THUMarkLau May 31, 2024

Caideyipi May 28, 2024

THUMarkLau May 31, 2024

Caideyipi May 28, 2024 •

edited

THUMarkLau Jun 7, 2024

jt2594838 left a comment

jt2594838 May 31, 2024

THUMarkLau May 31, 2024

jt2594838 Jun 4, 2024 •

edited

THUMarkLau Jun 9, 2024

jt2594838 Jun 12, 2024

jt2594838 May 31, 2024

THUMarkLau Jun 9, 2024

jt2594838 Jun 12, 2024 •

edited

jt2594838 May 31, 2024

THUMarkLau Jun 9, 2024

jt2594838 Jun 12, 2024

jt2594838 Jun 4, 2024

THUMarkLau Jun 7, 2024

jt2594838 Jun 12, 2024

OneSizeFitsQuorum Jun 11, 2024

OneSizeFitsQuorum Jun 11, 2024

sonarcloud bot commented Jun 12, 2024

jt2594838 left a comment

jt2594838 Jun 12, 2024

jt2594838 Jun 12, 2024

		// Do not compress it
		IoTDBDescriptor.getInstance().getConfig().setWALCompressionAlgorithm(CompressionType.LZ4);

Support WAL Compression #12476

Are you sure you want to change the base?

Support WAL Compression #12476

Conversation

THUMarkLau commented May 7, 2024 • edited

codecov bot commented May 25, 2024 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Caideyipi May 28, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jt2594838 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jt2594838 Jun 4, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jt2594838 Jun 12, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jun 12, 2024

Quality Gate failed

jt2594838 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

THUMarkLau commented May 7, 2024 •

edited

codecov bot commented May 25, 2024 •

edited

Caideyipi May 28, 2024 •

edited

jt2594838 Jun 4, 2024 •

edited

jt2594838 Jun 12, 2024 •

edited