feat: secondary rocksdb storage #281
Conversation
|
|
||
| /// Replicate recent changes from primary database | ||
| /// Available only for a secondary storage | ||
| pub fn try_to_catch_up_from_primary(&self) -> Result<(), Error> { |
There was a problem hiding this comment.
I've rarely seen any try_ naming except try_from and only because there is a non-try alternative (just from), the Result in the signature is explicit enough about possible failure
|
|
||
| self.commit_db_write_batch(db_batch, pending_costs, transaction) | ||
| .add_cost(cost) | ||
| fn create_checkpoint<P: AsRef<Path>>(&self, path: P) -> Result<(), Error> { |
There was a problem hiding this comment.
shall we even checkpoint on secondary?
There was a problem hiding this comment.
Technically you can do it. Do you think we shouldn't allow it?
There was a problem hiding this comment.
I don't know how you plan to split responsibilities between instances, and whether it works for secondary instances, it just caught my eye
| pub struct PrefixedSecondaryRocksDbStorageContext<'db> { | ||
| pub(in crate::rocksdb_storage) storage: &'db NonTransactionalDb, | ||
| pub(in crate::rocksdb_storage) prefix: SubtreePrefix, | ||
| pub(in crate::rocksdb_storage) batch: Option<&'db StorageBatch>, |
There was a problem hiding this comment.
do we need to have it for secondary?
There was a problem hiding this comment.
We reuse it in another context
| } | ||
|
|
||
| /// Create a new prefixed context instance | ||
| pub fn new_secondary( |
There was a problem hiding this comment.
it's context_tx file, we're dealing with transactions there and this one receives none as it should
so I suggest to have no such context at all because we have no such case for secondary
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only