Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache store generic #914

Closed
wants to merge 5 commits into from
Closed

Cache store generic #914

wants to merge 5 commits into from

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented May 9, 2024

A generic version of #897 using the storage abstractions instead of rocksdb directly.

The PR isn't finished, I just wanted to see what were the problems that stopped @fridrik01 from using the KVStore. I won't lie it's a lot of generics; in a way the syncer is the one of the worst places to introduce this kind of stuff because it's layers and layers of indirection.

I tried to alleviate it a little by removing the S parameter from App. I imagine it might help to try to wrap things up into abstractions that do a specific cache functionality, rather than having to pass around a KVStore parameter together with all the things that it needs to be able to serialize. But I wanted to do this to show the way it can be done.

Had to refactor STM a bit in aakoshh/async-stm-rs@113be84 to get around a compilation issue. This now demonstrates how to use RocksDB transactions together with STM ones. There might be ways to refactor the panicky commit/rollback, just need to have another long look at how they are used.

@aakoshh aakoshh mentioned this pull request May 10, 2024
@raulk raulk closed this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants