[Release-7.1] Add auto range split for hot storage queue#10932
Conversation
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
f8b3351 to
210413f
Compare
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
210413f to
6535309
Compare
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
6535309 to
d18aa83
Compare
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
d18aa83 to
55bdb05
Compare
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
362dd63 to
928a90c
Compare
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
928a90c to
47c4a93
Compare
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
dlambrig
left a comment
There was a problem hiding this comment.
Can you add a note describing if you saw your new logs after running simulation tests?
| init( ENABLE_AUTO_SHARD_SPLIT_FOR_LONG_STORAGE_QUEUE, false ); if( randomize && BUGGIFY ) ENABLE_AUTO_SHARD_SPLIT_FOR_LONG_STORAGE_QUEUE = true; | ||
| init( DD_SS_TOO_LONG_STORAGE_QUEUE_BYTES, TARGET_BYTES_PER_STORAGE_SERVER*0.5); if( randomize && BUGGIFY ) DD_SS_TOO_LONG_STORAGE_QUEUE_BYTES = TARGET_BYTES_PER_STORAGE_SERVER*0.05; | ||
| init( DD_SS_SHORT_STORAGE_QUEUE_BYTES, TARGET_BYTES_PER_STORAGE_SERVER*0.35); if( randomize && BUGGIFY ) DD_SS_SHORT_STORAGE_QUEUE_BYTES = TARGET_BYTES_PER_STORAGE_SERVER*0.035; | ||
| init( DD_STORAGE_QUEUE_TOO_LONG_DURATION, 300.0 ); if( isSimulated ) DD_STORAGE_QUEUE_TOO_LONG_DURATION = deterministicRandom()->random01() * 10 + 1; |
There was a problem hiding this comment.
curious why 300 sec chosen for the duration, and 60 sec for the split interval ? (seem like a long time)
There was a problem hiding this comment.
If a Storage Queue is long for 300 sec, we regard this Storage Queue is too long and updateAndGetStorageQueueTooLong will return true for each time the SS metric gets updated until the Queue becomes short. Consider a case where a SS Queue keeps long for 600 seconds. After the first 300 seconds, for each time the SS metric gets updated (every 2 seconds), updateAndGetStorageQueueTooLong will be called and return true. However, we do not want SS tracker gets triggered for each time updateAndGetStorageQueueTooLong returns true, which is too frequent. So, I add DD_MIN_LONG_STORAGE_QUEUE_SPLIT_INTERVAL_SEC.
| .detail("SSID", ssid) | ||
| .detail("StorageQueueBytes", currentBytes); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
I think this else branch could be removed to simplify, and line 118 become
if (currentBytes >= SERVER_KNOBS->DD_SS_TOO_LONG_STORAGE_QUEUE_BYTES) {
There was a problem hiding this comment.
There are two knobs: DD_SS_TOO_LONG_STORAGE_QUEUE_BYTES and DD_SS_SHORT_STORAGE_QUEUE_BYTES, where DD_SS_TOO_LONG_STORAGE_QUEUE_BYTES > DD_SS_SHORT_STORAGE_QUEUE_BYTES. In the first case, a storage queue is marked too long if it is longer than DD_SS_TOO_LONG_STORAGE_QUEUE_BYTES. In the second case, a storage queue is marked normal if it is shorter than DD_SS_SHORT_STORAGE_QUEUE_BYTES. In the third case, a storage queue state does not change.
47c4a93 to
8a9d6fd
Compare
Result of foundationdb-pr-macos-m1 on macOS Ventura 13.x
|
Result of foundationdb-pr-macos on macOS Ventura 13.x
|
Result of foundationdb-pr-clang on Linux CentOS 7
|
Result of foundationdb-pr on Linux CentOS 7
|
Result of foundationdb-pr-cluster-tests on Linux CentOS 7
|
jzhou77
left a comment
There was a problem hiding this comment.
Have some questions for clarifications.
| if (shardWriteTraffic > maxShardWriteTraffic && | ||
| shardWriteTraffic > SERVER_KNOBS->DD_MIN_SHARD_BYTES_PER_KSEC_TO_MOVE_OUT) { | ||
| shardToMove = it->range(); | ||
| selectedTeam = team; |
There was a problem hiding this comment.
Let's say you have selected a team, now you have another "team" goes here. Maybe you should compare these two teams and choose one?
There was a problem hiding this comment.
Current design is that the hot server will keep moving hot shards out of the server until (1) the server queue becomes short; or (2) all shards of the server are cold. The rate of moving out shards is at most one shard per minute. I am open to any scheduling design, and I am happy to discuss this further.
There was a problem hiding this comment.
Oh, I see that you use maxShardWriteTraffic to find the hottest team. That's good.
I have an idea of quarantine shards to separate SSes and don't throttle traffic if these SSes have long queue, which needs changes at both DD and RK. Will talk with you and @dlambrig about it.
There was a problem hiding this comment.
Yes. It would be great to cooperate DD and RK to optimize throughput. Look forward to discussing with you.
| if (shardWriteTraffic > maxShardWriteTraffic && | ||
| shardWriteTraffic > SERVER_KNOBS->DD_MIN_SHARD_BYTES_PER_KSEC_TO_MOVE_OUT) { | ||
| shardToMove = it->range(); | ||
| selectedTeam = team; |
There was a problem hiding this comment.
Oh, I see that you use maxShardWriteTraffic to find the hottest team. That's good.
I have an idea of quarantine shards to separate SSes and don't throttle traffic if these SSes have long queue, which needs changes at both DD and RK. Will talk with you and @dlambrig about it.
This PR automatically detects any storage server that has a long storage queue for long time and issue data moves out of the hot server to mitigate this issue.
DDServerTracker detects if a storage server has a long storage queue for a long time (300 seconds by default). If yes, DDServerTracker notifies DDTracker. Then DDTracker goes through all shards owned by the server and moves the shard with the highest write traffic out of the server. The minimal interval between two successive this type of data moves is 60 seconds by default.
Correctness test with feature off:
20231009-181137-zhewang-451afc8ece7567ca compressed=True data_size=24134350 duration=4613284 ended=100000 fail_fast=10 max_runs=100000 pass=100000 priority=100 remaining=0 runtime=0:28:26 sanity=False started=100000 stopped=20231009-184003 submitted=20231009-181137 timeout=5400 username=zhewang
Correctness test with feature on:
20231009-205149-zhewang-77f0df89c7859f29 compressed=True data_size=24134234 duration=5045529 ended=100000 fail=1 fail_fast=10 max_runs=100000 pass=99999 priority=100 remaining=0 runtime=0:33:02 sanity=False started=100000 stopped=20231009-212451 submitted=20231009-205149 timeout=5400 username=zhewang
Relevant trace events:
StorageQueueTooLongNotified
StorageQueueTooLongTriggerSplit
DDTrackerStorageServerQueueTooLongNotified
SendRelocateToDDQueue
Code-Reviewer Section
The general pull request guidelines can be found here.
Please check each of the following things and check all boxes before accepting a PR.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
release-branchormainif this is the youngest branch)