feat(sdk): Introduce linked_chunk::Updates
#3384
Merged
Conversation
This file contains 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
…hunkUpdates` type. This patch updates the `LinkedChunk::update_history` field from a simple `Vec<LinkedChunkUpdate<Item, Gap>>` type to the new `LinkedChunkUpdates<Item, Gap>` type (note the plural). This is going to be helpul for the next patches.
Hywan
force-pushed
the
feat-sdk-observable-linked-chunk
branch
from
60cfc29 to
12bc2ed
Compare
This patch adds the `LinkedChunkUpdates::peek` method. This is a new channel to read the updates without consuming them, like `LinkedChunkUpdates::take` does. The complexity is: when do we clear/drop the updates then? We don't want to keep them in memory forever. Initially `take` was clearing the updates, but now that we can read them with `peek` too, who's responsible to clear them? Enter `garbage_collect`. First off, we already need to maintain 2 index, resp. `last_taken_index` and `last_peeked_index` so that `take` and `peek` don't return already returned updates. They respectively know the index of the last update that has been read. We can use this information to know which updates must be garbage collected: that's all updates below the two index. Tadaa. Simple. The only _drawback_ (if it can be considered as such) is that the garbage collection happens on the next call to `take` or `peek` (because of the borrow checker). That's not really a big deal in practise. We could make it happens immediately when calling `take` or `peek` but it needs more pointer arithmetic and a less straighforward code.
Hywan
force-pushed
the
feat-sdk-observable-linked-chunk
branch
from
12bc2ed to
d691579
Compare
`LinkedChunkUpdate` implements `Clone` if and only if `Item` and `Gap` both implement `Clone`.
This patch implements `LinkedChunkUpdates::subscribe` and `LinkedChunkUpdateSubscriber`, which itself implements `Stream`. This patch splits `LinkedChunkUpdates` into `LinkedChunkUpdatesInner`, so that the latter can be shared with `LinkedChunkUpdatesSubscriber`.
Hywan
force-pushed
the
feat-sdk-observable-linked-chunk
branch
from
0db7664 to
f492dfa
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3384 +/- ##
==========================================
+ Coverage 83.14% 83.21% +0.07%
==========================================
Files 243 244 +1
Lines 25196 25238 +42
==========================================
+ Hits 20950 21003 +53
+ Misses 4246 4235 -11 ☔ View full report in Codecov by Sentry. |
Hywan
changed the title
LinkedChunkUpdateslinked_chunk::Updates
This patch removes the notion of `take` vs. `peek` from `LinkedChunkUpdatesInner` and widens the problem to a more general approach: `LinkedChunkUpdatesInner` must support multiple readers, not only two (`take` was the first reader, `peek` was the second reader, kind of). Why do we need multiple readers? `LinkedChunkUpdates::take` is clearly the first reader, it's part of the public API. But the private API needs to read the updates too, without consuming them, like `LinkedChunkUpdatesSubscriber`. `peek` was nice for that, but it's possible to have multiple `LinkedChunkUpdatesSubscriber` at the same time! Hence the need to widen the approach from 2 readers to many readers. This patch introduces a `ReaderToken` to identify readers. The last indexes are now all stored in a `HashMap<ReaderToken, usize>`. The rest of the modifications are the consequence of that. The test `test_updates_take_and_peek` has been entirely rewritten to be `test_updates_take_and_garbage_collector` where it tests 2 readers and see how the garbage collector reacts to that.
Hywan
force-pushed
the
feat-sdk-observable-linked-chunk
branch
from
f492dfa to
1493dc2
Compare
This was referenced May 8, 2024
andybalaam
approved these changes
May 9, 2024
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.
This patch introduces
linked_chunk::Updates, which is a new type to handle all updates of aLinkedChunk.Updatesis basically a wrapper aroundVec<Update>: aLinkedChunkemitsUpdates, that can be retrieved/read byUpdatesbasically.Updateis useful for replicating theLinkedChunkin a database for example, or for creating streams like the yet-to-be-implementedLinkedChunk::subscribe_as_vector.This patch has started with 2 methods on
Updates:taketo read and to consume updates (that's useful for databases) andpeekto read updates without consuming them (that's useful for subscriber, and laterLinkedChunk::subscribe_as_vector). However, it has changed during the development of this patch to a multi-readers API. The code and the commit messages explain the whys and hows in details.This patch finally introduces
linked_chunk::UpdateSubscriber, which implementsStream. That's the foundation ofLinkedChunk::subscribe_as_vector.My advice is to review this PR commit by commit, it will be much easier.
EventCachestorage #3280