feat(event cache): leaner updates when clearing/shrinking to the last chunk #4709
Conversation
…king a linked chunk
…ing a linked chunk
| pub async fn reset(&mut self) -> Result<Vec<VectorDiff<TimelineEvent>>, EventCacheError> { | ||
| pub async fn reset( | ||
| &mut self, | ||
| sync_timeline_events_diffs: &mut Vec<VectorDiff<TimelineEvent>>, |
There was a problem hiding this comment.
Why do you pass a &mut instead of keeping to return the diffs?
There was a problem hiding this comment.
This was to follow the pattern introduced in replace_with, which makes use of this function. I can isolate using the pattern to replace_with / shrink_to_last_chunk, if you'd prefer, and have the override happen there?
There was a problem hiding this comment.
I'm struggling to see how it will look like but if the idea is to remove this &mut, I think I would prefer that yes!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4709 +/- ##
=======================================
Coverage 85.94% 85.95%
=======================================
Files 290 290
Lines 33892 33906 +14
=======================================
+ Hits 29130 29145 +15
+ Misses 4762 4761 -1 ☔ View full report in Codecov by Sentry. |
I wanted to look at what it'd take to implement the
TODOs about clearing, and I think I got them right, in the first commit; the only thing to pay attention to was thelast_index_per_readerthat needed an update when clearing updates.But this wasn't sufficient to actually get rid of the spurious updates I've introduced in a previous test, because we're extending the previous updates with the ones caused by the shrink/reset. Instead, I've tried passing the updates diff vector as a
&mutparameter to reset/shrink etc., but it makes the APIs a bit uglier, but eventually it behaves as I expected. What do you think @Hywan?