core: Provide read access to commit/message log and odb #265
Conversation
This is a first approximation to provide data access for Kafka-style replication. It punts on notifications of segment rotation (which could also be solved via filesystem events). ObjectDB access is fairly crude, and will likely require it's own replication subsystem.
|
This will probably do, so rebased into |
| /// The iterator represents a _snapshot_ of the log at the time this method | ||
| /// is called. That is, segments created after the method returns will not | ||
| /// appear in the iteration. The last segment yielded by the iterator may be | ||
| /// incomplete (i.e. still be appended to). |
There was a problem hiding this comment.
These doc comments are A+ 🙏
|
|
||
| /// A read-only view of a [`CommitLog`]. | ||
| pub struct CommitLogView { | ||
| mlog: Option<Arc<Mutex<MessageLog>>>, |
There was a problem hiding this comment.
I might suggest implementing the CommitLog by giving it a CommitLogView internally, since it does already share the same fields. Hmm that doesn't seem like the right solution either, but I wonder if we can have a clearer abstraction here.
It seems like we are almost trying to create the concept of a "File" (i.e. CommitLogView) and a "BufferedFile" (i.e. CommitLog) where the buffered thing is unwritten commit which is only periodically flushed. I wonder if something like that might fit better down the road.
There was a problem hiding this comment.
The purpose of CommitLogView is for RelationalDB to hand out a view over its internal state in order to serve up log segments. It must hide append, but changing the visibility of append isn't helpful.
CommitLogView could be thought of as a newtype around CommitLog, but internally that'd be copying more data than strictly required. Perhaps if there was an Arc around the whole CommitLog, but I'm always wary about contention.
| /// The underlying file is opened lazily when calling [`SegmentView::try_into_iter`] | ||
| /// or [`SegmentView::try_into_file`]. | ||
| #[derive(Clone, Debug)] | ||
| pub struct SegmentView { |
There was a problem hiding this comment.
Why is this required instead of MessageLogIter? Also presumably we should implement the internals of MessageLogIter with SegementView because we're duplicating some code that really needs to stay in sync.
There was a problem hiding this comment.
I was going to ask about MessageLogIter, and perhaps submit a separate patch: the issue is, it has very different semantics. For one, it panics on I/O errors, which isn't great when it's not a matter of database consistency. More importantly, though, segments_for_offset returns the first segment if offset does not fall into the last segment -- the significance of which I don't understand.
So imho this would require to write a number of tests at least, which I wouldn't want to conflate with this patch (which does not change any database internals).
There was a problem hiding this comment.
Why is this required instead of MessageLogIter?
It is also not helpful to borrow MessageLog when that value is behind a mutex.
There was a problem hiding this comment.
Oh and also note that it is useful to yield whole segments (in addition to a stream of messages) to allow consumers which are very far behind to fetch all of them in parallel.
Also make max segment size configurable, so tests don't have to write >= 1GiB worth of data.
Description of Changes
This is a first approximation to provide data access for Kafka-style replication. It punts on notifications of segment rotation (which could also be solved via filesystem events).
ObjectDB access is fairly crude, and will likely require it's own replication subsystem.
Note that this is againstcloud-next.API and ABI
If the API is breaking, please state below what will break