feat: pipeline loop #10

onbjerg · 2022-10-03T15:28:09Z

Implements the execution and unwinding loops of the pipeline. The saving/loading of stage progress is also implemented, although it's not nice to look at w/o db abstractions

onbjerg · 2022-10-03T15:29:02Z

crates/stages/src/pipeline.rs

 ///
 /// The unwind priority is set with [Pipeline::push_with_unwind_priority]. Stages with higher unwind
 /// priorities are unwound first.
 #[derive(Default)]
 pub struct Pipeline {
    stages: Vec<QueuedStage>,
-    unwind_to: Option<U64>,


Can just call unwind directly

onbjerg · 2022-10-03T15:29:12Z

crates/stages/src/pipeline.rs

    max_block: Option<U64>,
-    exit_after_sync: bool,


Essentially the same as max_block so I just deduplicated

rkrasiuk

left some comments

rkrasiuk · 2022-10-05T04:34:05Z

crates/stages/src/pipeline.rs

+        let mut unwind_pipeline = {
+            let mut stages: Vec<_> = self.stages.iter_mut().enumerate().collect();
+            stages.sort_by_key(|(id, stage)| {
+                if stage.unwind_priority > 0 {
+                    (id - stage.unwind_priority, 0)
+                } else {
+                    (*id, 1)
+                }
+            });
+            stages.reverse();
+            stages
+        };


there is a possible overflow since id & unwind_priority are usizes. in general, the sorting here is a bit confusing. let's say, we have the queue of [stage1, stage2, stage3, stage4] and we want the resulting unwind queue to be [stage4, stage1, stage2, stage3], so we assign priorities stage3 - 1, stage2 - 2, stage1 - 3. if we were to add stage5 at arbitrary place, wouldn't we need to modify all of the existing priorities?

That's correct. I've been thinking about a better way to do this as well

rkrasiuk · 2022-10-05T04:40:40Z

crates/stages/src/pipeline.rs

+                                rx.send(PipelineEvent::Ran { stage_id, result: None }).await?
+                            }
+
+                            return Err(e)


hmm, any additional error handling we'd need to do here? just making sure that if it's a fatal error, on next restart the pipeline should recover the previous state from the db

We should never encounter fatal errors that result in unwinds, those are bugs that we should fix as it alludes to something major has gone wrong. If an unwind is necessary the stage should specify that

If we encounter this branch then the current transaction is aborted, so it will sync from the point where the transaction was committed last. If we want to go further back than that then I'm not sure how we would determine that without the stage telling us since we don't track a history of stage progress

perfect, ty 👌

mattsse

I'm lacking a bit of context here and it's not obvious what the pipeline does.

mattsse · 2022-10-05T09:31:27Z

crates/stages/src/lib.rs

@@ -60,18 +62,15 @@ pub enum StageError {
    ///
    /// TODO: This depends on the consensus engine and should include the validation failure reason
    #[error("Stage encountered a validation error.")]


include block number in error?

mattsse · 2022-10-05T09:34:21Z

crates/stages/src/pipeline.rs

    /// The actual stage to execute.
-    stage: Box<dyn Stage>,
+    stage: Box<dyn Stage<'db, E>>,
    /// The unwind priority of the stage.
    unwind_priority: usize,
    /// Whether or not this stage can only execute when we reach what we believe to be the tip of


tip of tip?

Unsure what this refers to, it doesn't say tip of tip?

there's another tip on the next line -.-
github annotations for unchanged lines could use some work

mattsse · 2022-10-05T09:34:41Z

crates/stages/src/pipeline.rs

+    /// Create a new pipeline.
+    pub fn new() -> Self {
+        Default::default()
+    }


redundant new, can just use default

mattsse · 2022-10-05T09:36:09Z

crates/stages/src/pipeline.rs

+                        .map_or(false, |(progress, target)| progress >= target);
+
+                    // Execute stage
+                    let output = async {


does it make sense to move this to a separate function?

Unsure, it won't really be used outside of this loop. If you just want to execute a stage directly you would call Stage::execute

onbjerg · 2022-10-05T14:54:49Z

I'm lacking a bit of context here and it's not obvious what the pipeline does.

Do you have ideas for how to improve the rustdoc then? It just executes the stages serially, and once it has looped through all of them, it executes them again. It's the main driver in staged syncing. If a stage encounters a validation error, it unwinds the stages in reverse order and syncs again. That's all it does

mattsse

I see, was just lacking the context on Pipeline/Stage in general.

should merge and iterate over with follow ups?

onbjerg · 2022-10-06T16:24:39Z

Yeah let's merge and think of a better unwind priority system

…ve-import chore(executor): removed useless import

Resolution checkpoint Resolution checkpoint paradigmxyz#2 Resolution checkpoint paradigmxyz#3 x Resolution checkpoint paradigmxyz#4 Resolution checkpoint paradigmxyz#5 Resolution checkpoint paradigmxyz#6 Resolution checkpoint paradigmxyz#7 Resolution checkpoint paradigmxyz#8 Resolve checkpoint paradigmxyz#9 (transaction primitive) Resolve checkpoint paradigmxyz#10 (rpc api transactions) Resolve checkpoint paradigmxyz#11 (building w/o feature flag) Start review Compiling with and without `optimism` feature flag Remove `DepositTx` from txpool mock tests, they never go into the txpool fmt code lint fix signature tests Co-authored-by: nicolas <[email protected]> Use free CI runners (revert before upstream) Co-authored-by: refcell <[email protected]> Signature test fixes Co-authored-by refcell <[email protected]> Fix Receipt proptest Co-authored-by BB <[email protected]> lint Fix variable-length compact for txtype/transaction Co-authored-by: Brian Bland <[email protected]> Fix basefee tests Remove unnecessary rpc deps Co-authored-by: Brian Bland <[email protected]> Co-authored-by: refcell <[email protected]> Co-authored-by: nicolas <[email protected]> Co-authored-by: Roberto <[email protected]>

* feat: support opBNB * chore: update revm * fix: merge conflicts * feat: add opbnb features * chore: fix fmt * fixup! chore: fix fmt * fix: add opbnb features to more code spaces * feat: add mgasps log * ci: change ut runner to github official provided * fix: mgas ps log * ci: fix unit, integration test job * fix: revm spec dispatcher * chore: update revm * fix: add fermat fork timestamp to fix revm init * update revm and alloy-chain version --------- Co-authored-by: j75689 <[email protected]> Co-authored-by: Keefe Liu <[email protected]>

Add pre-round context to next round execution

onbjerg added 2 commits October 3, 2022 17:25

feat: block number in validation error

3423a7b

feat: wip pipeline execution and unwind

f50b0f0

onbjerg commented Oct 3, 2022

View reviewed changes

onbjerg added 9 commits October 3, 2022 17:30

chore: clippy

98f6a24

feat: remove dummy db tx in stages crate

3728b57

feat: stage progress saving and loading

e38ac0a

test: wip tests

ffd324c

feat: basic tracing in pipeline

17e3094

feat: pipeline event channel

155c3a2

test: wip more tests

17f6284

test: test unwind during run

6496c65

chore: readability shuffle

0e5ad0d

onbjerg changed the title ~~wip: pipeline loop~~ feat: pipeline loop Oct 4, 2022

onbjerg requested review from mattsse and rkrasiuk and removed request for mattsse October 4, 2022 22:41

rkrasiuk reviewed Oct 5, 2022

View reviewed changes

mattsse reviewed Oct 5, 2022

View reviewed changes

mattsse approved these changes Oct 6, 2022

View reviewed changes

chore: incl block number in validation error

de8b720

onbjerg merged commit c749658 into master Oct 6, 2022

onbjerg deleted the onbjerg/pipeline-loop branch October 6, 2022 16:24

BrianBland pushed a commit to BrianBland/reth that referenced this pull request Jul 13, 2023

Merge pull request paradigmxyz#10 from refcell/merklefruit/chore-remo…

f689ac2

…ve-import chore(executor): removed useless import

anonymousGiga added a commit to anonymousGiga/reth that referenced this pull request Feb 20, 2024

use minstant instead of TimeRecord (paradigmxyz#10)

72cb604

AshinGau added a commit to AshinGau/reth that referenced this pull request Sep 20, 2024

grevm: add pre-round context to next round execution (paradigmxyz#10)

9b2c8fb

Add pre-round context to next round execution

AshinGau added a commit to AshinGau/reth that referenced this pull request Oct 13, 2024

grevm: add pre-round context to next round execution (paradigmxyz#10)

4951b8d

Add pre-round context to next round execution

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: pipeline loop #10

feat: pipeline loop #10

onbjerg commented Oct 3, 2022 •

edited

Loading

onbjerg Oct 3, 2022

onbjerg Oct 3, 2022

rkrasiuk left a comment

rkrasiuk Oct 5, 2022

onbjerg Oct 6, 2022

rkrasiuk Oct 5, 2022

onbjerg Oct 5, 2022 •

edited

Loading

rkrasiuk Oct 5, 2022

mattsse left a comment

mattsse Oct 5, 2022

mattsse Oct 5, 2022

onbjerg Oct 5, 2022

mattsse Oct 5, 2022

mattsse Oct 5, 2022

mattsse Oct 5, 2022

onbjerg Oct 5, 2022

onbjerg commented Oct 5, 2022

mattsse left a comment

onbjerg commented Oct 6, 2022

feat: pipeline loop #10

feat: pipeline loop #10

Conversation

onbjerg commented Oct 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rkrasiuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onbjerg Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onbjerg commented Oct 5, 2022

mattsse left a comment

Choose a reason for hiding this comment

onbjerg commented Oct 6, 2022

onbjerg commented Oct 3, 2022 •

edited

Loading

onbjerg Oct 5, 2022 •

edited

Loading