RFC: reduce traffic for resolved ts #108
Conversation
Signed-off-by: you06 <you1474600@gmail.com>
you06
force-pushed
the
reduce-traffic-resolved-ts
branch
from
d005adb
to
2fb55da
Compare
|
|
||
| The proposed method for reducing the traffic of pushing safe ts is hibernated region optimization. This method can be especially beneficial for large clusters since it can save a significant amount of traffic. | ||
|
|
||
| The motivation behind this approach is that TiKV currently uses the `CheckLeader` request to push resolved ts, and the traffic it costs grows linearly with the number of regions. By utilizing hibernated region optimization, it is possible to reduce this traffic and improve overall performance. |
There was a problem hiding this comment.
But hibernated region is removed in raftstore-v2
There was a problem hiding this comment.
Yeah, maybe the traffic of pushing safe ts won't be a problem if the region size is dynamic. This RFC is only an optimization in raftstore-v1.
Signed-off-by: you06 <you1474600@gmail.com>
Signed-off-by: you06 <you1474600@gmail.com>
Signed-off-by: you06 <you1474600@gmail.com>
you06
force-pushed
the
reduce-traffic-resolved-ts
branch
from
ac74ce1
to
5890509
Compare
| @@ -0,0 +1,114 @@ | |||
| ## Summary | |||
|
|
|||
| The proposed method for reducing the traffic of pushing safe ts is hibernated region optimization. This method can be especially beneficial for large clusters since it can save a significant amount of traffic. | |||
There was a problem hiding this comment.
The proposed method for reducing the traffic of pushing safe ts is hibernated region optimization.
Currently, the main strategy in the new design is to minimize leader validation traffic and safe-ts pushing traffic for valid leaders.
| The `CheckLeaderResponse` respond with the regions that pass the Raft safety check. The leader can then push its `safe_ts` for those regions. Since most regions will pass the safety check, it is not necessary to respond with the IDs of all passing regions. Instead, we can respond with only the IDs of regions that fail the safety check. | ||
|
|
||
| Another optimization is that we can confirm the leadership if the leader lease is hold, by calling Raft's read-index command. But this will involve in the Raftstore thread pool, more CPU will be used by this. | ||
|
|
There was a problem hiding this comment.
We could add some explanations:
By default, the raft lease is set to 10s. If the advance-ts-interval is configured to 100ms, only one leader validation using the read index method will be required for 100 rounds which saves 99 rounds of unnecessary leader checks.
There was a problem hiding this comment.
Does it mean that in this case we don't need to send CheckLeaderRequest every 100ms and can only send ApplySafeTsRequest while we are under the lease?
There was a problem hiding this comment.
Yes as the validation and pushing are separated into two phases.
| ``` | ||
|
|
||
| To save traffic, `ApplySafeTsRequest.unsafe_regions` only contains the regions whose leader may be changed. In the ideal case, this request is small because there is almost no unsafe regions. | ||
|
|
There was a problem hiding this comment.
This is an example that will make it easier for readers to understand. Like when advance-ts-interval is set to 100ms, how many bytes could be saved running 100 rounds comparing the new protocol and the previous one.
| Safety is guaranteed as long as the safe ts is generated from a **valid leader**. By decoupling the pushing of safe ts into two phases, the leader peer can ensure that the leadership is still unchanged before safe ts is generated. | ||
|
|
||
| For inactive regions, only the region IDs are sent. If the term has changed since the last active region check, the follower will respond with a check failure. When the leader receives the `CheckLeaderResponse`, and the inactive region ID is not in `CheckLeaderResponse.failed_regions`, it means that the terms of the leader and follower are both unchanged. After checking leadership for inactive regions, it's safe to push the safe timestamps from the leader. Additionally, since there are no following writes in inactive regions, the `applied_index` check is unnecessary. | ||
|
|
There was a problem hiding this comment.
Better to explain How safety is guaranteed if the leader is a stale one or the pushing request is from a stale one when the network partition happens.
| Another optimization is that we can confirm the leadership if the leader lease is hold, by calling Raft's read-index command. But this will involve in the Raftstore thread pool, more CPU will be used by this. | ||
|
|
||
| ### Inactive Regions | ||
|
|
There was a problem hiding this comment.
We need to explain how this new design interacts with the existing hibernated region feature and identify any dependencies it may have. Additionally, we should outline how to determine if a region is inactive.
As I understand it, the current workflow involves:
- Checking if a region leader is active.
- Validating that the leader role is still held for both active and inactive regions using different methods.
- Sending pushing ts requests; note that different request types are required for active and inactive regions.
Signed-off-by: you06 <you1474600@gmail.com>
| } | ||
|
|
||
| message ApplySafeTsResponse { | ||
| uint64 ts = 1; |
There was a problem hiding this comment.
Why do we need get a ts back in response? Also would we need ts for each active region in this case not just one ts?
There was a problem hiding this comment.
The ts is actually not used.
would we need ts for each active region in this case not just one ts?
No, because leader doesn not need to know the safe ts in followers.
|
|
||
| ```diff | ||
| message CheckLeaderRequest { | ||
| repeated LeaderInfo regions = 1; |
There was a problem hiding this comment.
If CheckLeaderRequest does not advance safe_ts anymore (as subsequent ApplySafeTsRequest does it), we probably can strip some data out of LeaderInfo as well. E.g. no need to pass ReadState anymore?
There was a problem hiding this comment.
We send LeaderInfo without ReadState as a transition state from active to inactive(you can see "Transition State" section in the updates).
For active regions(which have continues write), they are not contains in CheckLeaderRequest.regions, because the leadership is checked by Raft lease, we send the checked state of them in ApplySafeTsRequest.checked_leaders.
| ```protobuf | ||
| message CheckedLeader { | ||
| uint64 region_id = 1; | ||
| ReadState read_state = 2; |
There was a problem hiding this comment.
Do we need both AppliedIndex and SafeTs here? Anyway to save space further, we can introduce ReadStateDiff which would contain AppliedIndex and SafeTs diff with ApplySafeTsRequest.ts which would only take 2-4 bytes instead of 8 bytes. I believe uint64 is never a 8 bytes thanks to varint protobug encoding, but point is same. By passing only diff towards ApplySafeTsRequest.ts, it will always be much shorter and thanks to varint encoding we can save lot of space.
There was a problem hiding this comment.
Do we need both
AppliedIndexandSafeTshere?
Yes , it's contained in ReadState, because in active regions, the applied index in follower may delay from leader, we cannot apply the safe ts until the apply index catches up CheckedLeader.read_state.applied_index.
Anyway to save space further, we can introduce
ReadStateDiff
That's fun and I'll do some research of sending diff when we continue to push this project.
Signed-off-by: you06 <you1474600@gmail.com>
| } | ||
| ``` | ||
|
|
||
| To apply safe ts for valid leaders as soon as possible, instead of waiting for the next round of advancing resolved timestamps, we need to send another `ApplySafeTS` request. The `ApplySafeTS` request is usually small, the traffic caused by it can be ignored. |
There was a problem hiding this comment.
The
ApplySafeTSrequest is usually small
How small is it? Does it grow linearly with the number of regions?
There was a problem hiding this comment.
The size of ApplySafeTS request grows by the number of active regions whose the apply index is changed.
But we just move the leader info from CheckLeader request to ApplySafeTS request, even all regions are active, there won't be more traffic.
| message ApplySafeTsRequest { | ||
| uint64 ts = 1; | ||
| uint64 store_id = 2; | ||
| repeated uint64 unsafe_regions = 3; | ||
| repeated CheckedLeader checked_leaders = 4; | ||
| } |
There was a problem hiding this comment.
Could you add some comments about these fields?
|
|
||
| ### Active Regions | ||
|
|
||
| The `CheckLeaderResponse` respond with the regions that pass the Raft safety check. The leader can then push its `safe_ts` for those regions. Since most regions will pass the safety check, it is not necessary to respond with the IDs of all passing regions. Instead, we can respond with only the IDs of regions that fail the safety check. |
There was a problem hiding this comment.
Instead, we can respond with only the IDs of regions that fail the safety check.
What if messages get out of order? We may miss some failed regions.
Rendered