Bundle Account Locker and other refactors #54
Conversation
buffalu
changed the title
buffalu
changed the title
buffalu
requested review from
whistlinwilly,
jstarry,
jedleggett and
segfaultdoc
core/src/banking_stage.rs
Outdated
| @@ -417,7 +418,8 @@ impl BankingStage { | |||
| gossip_vote_sender: ReplayVoteSender, | |||
| cost_model: Arc<RwLock<CostModel>>, | |||
| connection_cache: Arc<ConnectionCache>, | |||
| tip_manager: Arc<Mutex<TipManager>>, | |||
| tip_manager: TipManager, | |||
There was a problem hiding this comment.
we use the lock() method to lock
core/src/banking_stage.rs
Outdated
| @@ -1113,7 +1123,8 @@ impl BankingStage { | |||
| data_budget: &DataBudget, | |||
| cost_model: Arc<RwLock<CostModel>>, | |||
| connection_cache: Arc<ConnectionCache>, | |||
| tip_manager: Arc<Mutex<TipManager>>, | |||
| tip_manager: TipManager, | |||
There was a problem hiding this comment.
how about just injecting with tip_program pubkey here? none of the other methods are used on this object
There was a problem hiding this comment.
good call, will fix that
| pub batch: PacketBatch, | ||
| pub uuid: Uuid, |
There was a problem hiding this comment.
Would it be useful to have this be a deterministic id? Something like hash of all tx signatures?
There was a problem hiding this comment.
doesn't matter too much i think, mainly just a way for tracking. imagine we can include this in metrics and cross-reference to our system at some point
| num_bundle_batches_prelock: u64, | ||
| unlocked_bundles: VecDeque<PacketBundle>, | ||
| locked_bundles: VecDeque<LockedBundle>, | ||
| read_locks: HashMap<Pubkey, u64>, |
There was a problem hiding this comment.
wdyt about defining these as HashMap<Rc<Pubkey>, u64> since these are copies of LockedBundle locks
core/src/bundle_account_locker.rs
Outdated
| } | ||
|
|
||
| pub struct BundleAccountLocker { | ||
| num_bundle_batches_prelock: u64, |
There was a problem hiding this comment.
leaving the comment here instead of throughout - this is a magic number to the new function in many cases - lets make a constant
There was a problem hiding this comment.
added constant in tpu.rs that gets passed in
The commit enables chained Merkle shreds for ClusterType::Development so that we get ample coverage for further development.
Problem
It's possible that BankingStage can execute transactions that interfere with transactions in a bundle that have already executed or not already executed. This would cause the validator to produce an invalid block or different blockhash than the rest of the network after replaying due to invalid account cache state.
Furthermore, it's possible that BundleStage may get blocked on AccountInUse from BankingStage when executing a bundle. In order to minimize this, we will pre-lock those accounts.
Summary of Changes
Bank::prepare_sanitized_batch_with_resultsTODO