Clickhouse backup on utils #780
Conversation
| /// Whether to use only the in-memory backup (for testing purposes). | ||
| #[cfg(test)] | ||
| use_only_memory_backup: bool, | ||
| _metrics_phantom: std::marker::PhantomData<MetricsType>, |
There was a problem hiding this comment.
Interesting, how are you planning to use this? I mean the Metrics trait and the marker here.
There was a problem hiding this comment.
Why is this inside a metrics folder? This is more time-related utilities.
There was a problem hiding this comment.
What's the rationale for distinguish clickhouse and clickhouse_with_backup? Eventually, I get it could make sense to have a backup subfolder of clickhouse.
There was a problem hiding this comment.
In orderflow proxy v2, this has been extracted from reth_tasks so that the project didn't have reth dependencies (at least directly). However, rbuilder heavily depends on reth already. As such, I'd suggest you to import reth_tasks in this project and use that instead. Then, you can delete this folder entirely.
There was a problem hiding this comment.
In more detail, rbuilder should re-export reth_tasks so that orderflow proxy v2 can use it, so there is no need to need to add it again as a dependency.
| pub trait Metrics { | ||
| fn increment_write_failures(err: String); | ||
| fn process_quantities(quantities: &Quantities); | ||
| fn record_batch_commit_time(duration: Duration); | ||
| fn increment_commit_failures(err: String); | ||
| fn set_queue_size(size: usize, order: &'static str); | ||
| fn set_disk_backup_size(size_bytes: u64, batches: usize, order: &'static str); | ||
| fn increment_backup_disk_errors(order: &'static str, error: &str); | ||
| fn set_memory_backup_size(size_bytes: u64, batches: usize, order: &'static str); | ||
| fn process_backup_data_lost_quantities(quantities: &Quantities); | ||
| fn process_backup_data_quantities(quantities: &Quantities); | ||
| fn set_backup_empty_size(order: &'static str); | ||
| } |
There was a problem hiding this comment.
You can expose a NoOp struct here with default implementation. Also another nit/question, do you explicitly want to not provide a default, empty implementation?
There was a problem hiding this comment.
In more detail, rbuilder should re-export reth_tasks so that orderflow proxy v2 can use it, so there is no need to need to add it again as a dependency.
2 participants
📝 Summary
Brought clickhouse robust saving (with local copy) from orderflow proxy repo (flashbots/buildernet-orderflow-proxy-v2.git)
💡 Motivation and Context
Code reuse
✅ I have completed the following steps:
make lintmake test