Add burst mode for congestion control #20631
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
aschran
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
aschran
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
aschran
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
| /// If >0, congestion control will allow transactions in total cost equaling the | ||
| /// configured amount to exceed the configured maximum accumulated cost per object. | ||
| /// As above, up to one transaction per object exceeding the burst limit will be allowed. | ||
| allowed_txn_cost_overage_burst_per_object_in_commit: Option<u64>, |
There was a problem hiding this comment.
when we discussed this last, we seemed to have settled on the mental model that the two params could be thought of as "the rate at which scheduling debt is paid down" and "the debt ceiling"
i think this model is much clearer, because i'm having trouble understanding your comments above even though I at least think I understand the system.
There was a problem hiding this comment.
it's slightly complicated by the requirement to maintain prior behavior when the new param is not set
max_accumulated_txn_cost_per_object_in_commit
- is always the rate at which debt is paid down
- also the absolute limit, if it's the only thing set
max_txn_cost_overage_per_object_in_commit
if set with 1,
- soft debt ceiling is 1
- absolute limit is 1+2
(this is how it is currently configured to work, with 2 == u64 max)
allowed_txn_cost_overage_burst_per_object_in_commit
if set with 1 and 2 (must be <= 2 per assert)
- soft debt ceiling is 1+3
- absolute limit is still 1+2
aschran
temporarily deployed
to
sui-typescript-aws-kms-test-env
GitHub Actions
Inactive
There was a problem hiding this comment.
LGTM! But I see there is an open discussion with @mystenmark , so probably should wait for him to take a look again.
| let absolute_limit = self | ||
| .max_accumulated_txn_cost_per_object_in_commit | ||
| .saturating_add(self.max_txn_cost_overage_per_object_in_commit); | ||
| if start_cost <= burst_limit && end_cost <= absolute_limit { |
There was a problem hiding this comment.
In a scenario where there is a consistent congestion, after the absolute_limit is hit and it starts to defer transactions, it won't wait until all the debts are paid off and then admit new transactions, but as soon as start_cost <= burst_limit is true, then it will admit new transactions, is that correct?
There was a problem hiding this comment.
Correct, we decided that method would be the hardest to attack.
PS. absolute_limit would never be hit with current settings, which have self.max_txn_cost_overage_per_object_in_commit set to u64::MAX
| @@ -2226,6 +2232,8 @@ impl ProtocolConfig { | |||
|
|
|||
| max_txn_cost_overage_per_object_in_commit: None, | |||
|
|
|||
| allowed_txn_cost_overage_burst_per_object_in_commit: None, | |||
There was a problem hiding this comment.
Is it expect that we don't set it yet in a protocol config?
There was a problem hiding this comment.
Yes, as a matter of principle I try to configure protocol config in a separate PR from the one that adds the feature
Description
Burst mode allows budget to be exceeded up to the configured burst limit.
Test plan
added/updated unit tests
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.