-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(wal): reduce concurrent conflicts between block write operations and poll operations #1554
Conversation
…and poll operations (AutoMQ#1550)
Hello, @RapperCL, you can update your code within the same PR; there's no need to create multiple PRs. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
s3stream/src/main/java/com/automq/stream/s3/wal/impl/block/SlidingWindowService.java
Outdated
Show resolved
Hide resolved
s3stream/src/main/java/com/automq/stream/s3/wal/impl/block/SlidingWindowService.java
Outdated
Show resolved
Hide resolved
s3stream/src/main/java/com/automq/stream/s3/wal/impl/block/SlidingWindowService.java
Outdated
Show resolved
Hide resolved
We have provided a tool called "WriteBench" for testing WAL performance. You can use it to compare the performance before and after the modifications to confirm their effectiveness. |
Okay, I'll find some time later to test it. |
…and poll operations (AutoMQ#1550)
…and poll operations (AutoMQ#1550)
s3stream/src/main/java/com/automq/stream/s3/wal/impl/block/SlidingWindowService.java
Outdated
Show resolved
Hide resolved
…and poll operations (AutoMQ#1550)
…and poll operations (AutoMQ#1550)
… and poll operations (AutoMQ#1550)
Hey, I conducted some simple local tests based on WriteBench on my side, and the test results are basically consistent with the expectations.:The performance improvement becomes more evident as the concurrency level increases. Here are the test data for two different scenarios: first: -p="D:\Users\chenyong152\AppData\Local\autmo\test2.log" -c=2000000000 -d=8 --iops=3000 --threads=1000 --throughput=100000000 --record-size=1000 --duration=600 second: Increase the number of threads to 5000 to simulate higher concurrency. |
This PR did indeed greatly improve the performance of WAL in many competitive scenarios. However, it also makes the correctness of WAL in a multi-concurrent context difficult for humans to understand. Considering that the number of concurrent writes in reality is usually consistent with the number of CPU cores, the current locking model is not a bottleneck for WAL performance. It is not recommended to introduce a complex locking model to avoid increasing maintenance costs. Is it possible to have better thread & concurrency models in the future for WAL to have both good performance and better correctness guarantees? (For example, single-thread or multi single-thread) |
In fact, previously, i had considered optimizing with multiple single-threaded models, but given the strong ordering guarantees required in the current scenario, it's difficult to isolate concurrent competition through this approach. I understand that optimizing with multiple single-threaded models is not suitable for scenarios that globally guarantee ordering, but is more suited to scenarios where local ordering is required. Based on the current write design, it is necessary to ensure the ordering of blocks during writing as well as the correctness of the This leads to the need for concurrent safety in managing competition among write operations, between write and poll operations, between poll and IO operations, as well as among IO operations. Previously, global ordering was ensured through After the current optimization: Within write operations, Based on the multiple single-threaded model, potential optimization points include:
Additionally, the introduction of |
… and poll operations (AutoMQ#1550)
@Chillax-0v0 @superhx PTAL Not only optimizations in performance but also enhancements in the threading model.
Adjusted Threading Model: The reactor master-slave threading model is now implemented as follows:
|
issue