-
Notifications
You must be signed in to change notification settings - Fork 622
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: resharding config allowing throttling #9994
Conversation
wacban
commented
Oct 24, 2023
- Added SplitStateConfig that configures the resharding batch size and delay between batches. Both can be used to throttle resharding.
- Added metrics to check how much time do different parts of resharding take.
@@ -579,6 +582,7 @@ impl Chain { | |||
pending_state_patch: Default::default(), | |||
requested_state_parts: StateRequestTracker::new(), | |||
state_snapshot_helper: None, | |||
state_split_config: StateSplitConfig::default(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The view client doesn't need to have resharding configured so it's fine to use default here.
// Once we build the iterator, we break it into batches using the get_trie_update_batch function. | ||
while let Some(batch) = get_trie_update_batch(&mut iter) { | ||
loop { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In here each of the three phases is wrapped in a dedicated scope with a relevant timer.
Slightly easier to review with whitespace hidden.
7c03720
to
caf37c5
Compare
|
||
RESHARDING_BATCH_COUNT.with_label_values(&metrics_labels).inc(); | ||
RESHARDING_BATCH_SIZE.with_label_values(&metrics_labels).add(size as i64); | ||
std::thread::sleep(config.batch_delay); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional: quick comment as to why we have this sleep (to ease out writes to RocksDB which is in contention with block processing..)
@@ -162,6 +162,24 @@ impl SyncConfig { | |||
} | |||
} | |||
|
|||
#[derive(serde::Serialize, serde::Deserialize, Clone, Copy, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be in near_config.config close to where StateSyncConfig
is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it might be fine here as we need to plumb it through to chain. That apparently only comes from chain config which comes from client config...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Apologies, the conflicts are from my PR 😅 |
caf37c5
to
721f904
Compare
- Added SplitStateConfig that configures the resharding batch size and delay between batches. Both can be used to throttle resharding. - Added metrics to check how much time do different parts of resharding take.