Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Don't send ActiveLeaves from leaves in db on startup in Overseer #6727

Merged
merged 6 commits into from
Mar 6, 2023
Merged

Don't send ActiveLeaves from leaves in db on startup in Overseer #6727

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5466659
Don't send `ActiveLeaves` from leaves in db on startup in Overseer. W…
tdimitrov Feb 16, 2023
5d6dc94
Don't pass initial set of leaves to Overseer
tdimitrov Feb 16, 2023
f6836a4
Fix compilation error in subsystem-test-helpers
tdimitrov Feb 16, 2023
d3cfc0c
Merge branch 'master' into tsv-no-db-leaves
tdimitrov Mar 2, 2023
4c52942
Merge branch 'master' into tsv-no-db-leaves
tdimitrov Mar 6, 2023
db7d858
Merge branch 'master' into tsv-no-db-leaves
tdimitrov Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions node/overseer/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,14 +728,10 @@ where
let metrics = self.metrics.clone();
spawn_metronome_metrics(&mut self, metrics)?;

// Notify about active leaves on startup before starting the loop
// Import active leaves on startup before starting the loop but don't send notifications for them
for (hash, number) in std::mem::take(&mut self.leaves) {
let _ = self.active_leaves.insert(hash, number);
if let Some((span, status)) = self.on_head_activated(&hash, None).await {
let update =
ActiveLeavesUpdate::start_work(ActivatedLeaf { hash, number, status, span });
self.broadcast_signal(OverseerSignal::ActiveLeaves(update)).await?;
}
self.on_head_activated(&hash, None).await;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this call actually do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and why do we care about stale leaves here?

Copy link
Contributor Author

@tdimitrov tdimitrov Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on_head_activated creates a span for the leaf and notifies external listeners (in case someone is subscribed for it). I think it's better not to remove it.

The entries it creates are cleaned up on finalization.

and why do we care about stale leaves here?

My theory: the initial idea was to start the subsystems as fast as possible. And there is a contract, that first leaf is special and it should be used just for a initialization. For example the dispute-coordinator waits for the first leaf before starting its main loop.

But at some point the subsystems started doing actual work when the first leaf is received and we started seeing the state discarded issues.

I misunderstood your question. After discussing it online - we don't care. I'll remove it.

}

loop {
Expand Down
30 changes: 0 additions & 30 deletions node/overseer/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,12 +410,6 @@ fn overseer_start_stop_works() {
handle.block_imported(third_block).await;

let expected_heartbeats = vec![
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf {
hash: first_block_hash,
number: 1,
span: Arc::new(jaeger::Span::Disabled),
status: LeafStatus::Fresh,
})),
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate {
activated: Some(ActivatedLeaf {
hash: second_block_hash,
Expand Down Expand Up @@ -509,18 +503,6 @@ fn overseer_finalize_works() {
handle.block_finalized(third_block).await;

let expected_heartbeats = vec![
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf {
hash: first_block_hash,
number: 1,
span: Arc::new(jaeger::Span::Disabled),
status: LeafStatus::Fresh,
})),
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf {
hash: second_block_hash,
number: 2,
span: Arc::new(jaeger::Span::Disabled),
status: LeafStatus::Fresh,
})),
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate {
deactivated: [first_block_hash, second_block_hash].as_ref().into(),
..Default::default()
Expand Down Expand Up @@ -606,18 +588,6 @@ fn overseer_finalize_leaf_preserves_it() {
handle.block_finalized(first_block).await;

let expected_heartbeats = vec![
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf {
hash: first_block_hash,
number: 1,
span: Arc::new(jaeger::Span::Disabled),
status: LeafStatus::Fresh,
})),
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate::start_work(ActivatedLeaf {
hash: second_block_hash,
number: 1,
span: Arc::new(jaeger::Span::Disabled),
status: LeafStatus::Fresh,
})),
OverseerSignal::ActiveLeaves(ActiveLeavesUpdate {
deactivated: [second_block_hash].as_ref().into(),
..Default::default()
Expand Down