Split storage per owner, drop StorageApi and MemStorage - #27
Merged
Conversation
jonasmartin
reviewed
Aug 7, 2026
| @@ -0,0 +1,110 @@ | |||
| // The BrokerServerStorage uses one key to store the uid and multiple keys to store the messages. | |||
Contributor
There was a problem hiding this comment.
Added a subtask in the current ongoing task to: Sanitize any input that will go into the db, specially to reject anything that contains the separator /
jonasmartin
approved these changes
Aug 7, 2026
jonasmartin
left a comment
Contributor
There was a problem hiding this comment.
It looks much clean now. Nice!
LGTM
- Added a sub task to the current task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split storage per owner, remove
StorageApiandMemStoragePreviously, two unrelated storage implementations lived behind a single
StorageApitrait.Treating them as interchangeable forced storage-related code to be generic when it didn't need to be.src/storage/now contains one concrete type for each use case:BrokerServerStorageBrokerNodeStorageThe only shared type is
BrokerStorageError.As a result,
LocalChannel,BrokerService,run, andBrokerServer::neware all concrete and no longer generic over storage.Removed the double lock
BrokerServerStoragenow takes&selfand performs its own internal locking. This allowsArc<Mutex<BrokerServerStorage>>to become simplyBrokerServerStoragePaths in, handles out
BrokerServer::newandBrokerNode::newnow take:and open the storage themselves.
LocalChannelownershipLocalChannelis now created by the server via:LocalChannel::newis nowpub(crate). This guarantees that a server and its channels always share the same storage handle.BrokerNodecleanupnode.rsis now 235 lines shorter. Many functions where moved intoBrokerNodeStorage.Key format changes
Server keys now use
/-separated paths, matching the node queue layout.Test cleanup
Two issues were fixed in the test infrastructure:
close()only signaled shutdown, so the server's RocksDB handle remained alive andremove_dir_all()failed silently.test_readme_examplenever cleaned up its temporary directory.