Skip to content
This repository has been archived by the owner on Dec 3, 2024. It is now read-only.

GC included txns #152

Merged
merged 4 commits into from
May 23, 2024
Merged

GC included txns #152

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e32da2
include timestamp
move47 May 22, 2024
e10f977
alternative approach for txn GC
move47 May 22, 2024
2f80f10
fixt lint
move47 May 22, 2024
bdacf4f
more idiomatic style
move47 May 23, 2024
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
21 changes: 16 additions & 5 deletions src/builder_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ pub struct BuilderState<TYPES: NodeType> {
>,

/// Included txs set while building blocks
pub included_txns: HashSet<Commitment<TYPES::Transaction>>,
pub included_txns: HashMap<Commitment<TYPES::Transaction>, TxTimeStamp>,

/// da_proposal_payload_commit to (da_proposal, node_count)
#[allow(clippy::type_complexity)]
Expand Down Expand Up @@ -239,7 +239,7 @@ impl<TYPES: NodeType> BuilderProgress<TYPES> for BuilderState<TYPES> {
let tx_hash = tx.commit();
tracing::debug!("Transaction hash: {:?}", tx_hash);
if self.tx_hash_to_available_txns.contains_key(&tx_hash)
|| self.included_txns.contains(&tx_hash)
|| self.included_txns.contains_key(&tx_hash)
{
tracing::debug!("Transaction already exists in the builderinfo.txid_to_tx hashmap, So we can ignore it");
} else {
Expand Down Expand Up @@ -270,7 +270,7 @@ impl<TYPES: NodeType> BuilderProgress<TYPES> for BuilderState<TYPES> {
// HOTSHOT MEMPOOL TRANSACTION PROCESSING
// If it already exists, then discard it. Decide the existence based on the tx_hash_tx and check in both the local pool and already included txns
if self.tx_hash_to_available_txns.contains_key(&tx_hash)
|| self.included_txns.contains(&tx_hash)
|| self.included_txns.contains_key(&tx_hash)
{
tracing::debug!("Transaction already exists in the builderinfo.txid_to_tx hashmap, So we can ignore it");
} else {
Expand Down Expand Up @@ -579,11 +579,22 @@ impl<TYPES: NodeType> BuilderProgress<TYPES> for BuilderState<TYPES> {
.for_each(|txn| {
if let Entry::Occupied(txn_info) = self.tx_hash_to_available_txns.entry(*txn) {
self.timestamp_to_tx.remove(&txn_info.get().0);
self.included_txns.insert(*txn);
self.included_txns.insert(*txn_info.key(), txn_info.get().0);
txn_info.remove_entry();
}
});

// clear the txns from included txns which are older than current timestamp by 3 secs
Copy link
Contributor

Choose a reason for hiding this comment

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

What? No. I meant, we could clear after... days? Hours, maybe, at the smallest increment.

Copy link
Contributor Author

@move47 move47 May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

2 hr?

Copy link
Contributor

@nyospe nyospe May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We can do 2 hours. But... maybe let's try this instead: https://espresso.zulipchat.com/#narrow/stream/419378-Block-Builder/topic/Staging.20performance.20issues/near/440153803 with the <unit of time> configurable, defaulting to 1 hour.

let current_timestamp = SystemTime::now()
.duration_since(SystemTime::UNIX_EPOCH)
.unwrap_or(Duration::new(0, 0))
.as_nanos();

let cutoff_time = current_timestamp - Duration::from_secs(3).as_nanos();

self.included_txns
.retain(|_txn, timestamp| *timestamp > cutoff_time);

// register the spawned builder state to spawned_builder_states in the global state
self.global_state.write_arc().await.register_builder_state(
self.built_from_proposed_block.vid_commitment,
Expand Down Expand Up @@ -926,7 +937,7 @@ impl<TYPES: NodeType> BuilderState<TYPES> {
BuilderState {
timestamp_to_tx: BTreeMap::new(),
tx_hash_to_available_txns: HashMap::new(),
included_txns: HashSet::new(),
included_txns: HashMap::new(),
built_from_proposed_block,
tx_receiver,
decide_receiver,
Expand Down
Loading