Repository navigation
CBG-5900: Add support to compact channel history with Channel Names #8857
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -288,8 +288,9 @@ func (c *DatabaseCollection) GetDocChannelHistory(ctx context.Context, docid str | |||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| // CompactDocChannelHistory removes channel history entries that ended at or before the given sequence number. | ||||||||||||||||
| // If seq is zero, it removes the history entries of the named channels instead. | ||||||||||||||||
| // This is used to prune stale channel assignment history to reduce storage overhead. | ||||||||||||||||
| func (c *DatabaseCollection) CompactDocChannelHistory(ctx context.Context, docid string, seq uint64) ([]string, error) { | ||||||||||||||||
| func (c *DatabaseCollection) CompactDocChannelHistory(ctx context.Context, docid string, seq uint64, channels []string) ([]string, error) { | ||||||||||||||||
| key := realDocID(docid) | ||||||||||||||||
| if key == "" { | ||||||||||||||||
| return nil, base.HTTPErrorf(400, "Invalid doc ID") | ||||||||||||||||
|
|
@@ -326,23 +327,42 @@ func (c *DatabaseCollection) CompactDocChannelHistory(ctx context.Context, docid | |||||||||||||||
| compactedChannels := make(base.Set) | ||||||||||||||||
|
|
||||||||||||||||
| doc.SyncData.ChannelSetHistory = slices.DeleteFunc(doc.SyncData.ChannelSetHistory, func(channel ChannelSetEntry) bool { | ||||||||||||||||
| del := channel.End <= seq | ||||||||||||||||
| var del bool | ||||||||||||||||
| if seq != 0 { | ||||||||||||||||
| del = channel.End <= seq | ||||||||||||||||
| } else if len(channels) > 0 { | ||||||||||||||||
| del = slices.Contains(channels, channel.Name) | ||||||||||||||||
|
Comment on lines
+330
to
+334
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this comment is valid, particularly given some of the known use cases related to large numbers of channels per doc - the iteration over channels to create the map wouldn't be significantly more expensive than a single slices.Contains, and you'd only need to do it once. |
||||||||||||||||
| } | ||||||||||||||||
| if del { | ||||||||||||||||
| compactedChannels.Add(channel.Name) | ||||||||||||||||
| } | ||||||||||||||||
| return del | ||||||||||||||||
| }) | ||||||||||||||||
|
|
||||||||||||||||
| doc.SyncData.ChannelSet = slices.DeleteFunc(doc.SyncData.ChannelSet, func(channel ChannelSetEntry) bool { | ||||||||||||||||
| del := channel.End != 0 && channel.End <= seq | ||||||||||||||||
| var del bool | ||||||||||||||||
| if seq != 0 { | ||||||||||||||||
| del = channel.End != 0 && channel.End <= seq | ||||||||||||||||
| } else if len(channels) > 0 { | ||||||||||||||||
| del = channel.End != 0 && slices.Contains(channels, channel.Name) | ||||||||||||||||
| } | ||||||||||||||||
| if del { | ||||||||||||||||
| compactedChannels.Add(channel.Name) | ||||||||||||||||
| } | ||||||||||||||||
| return del | ||||||||||||||||
| }) | ||||||||||||||||
|
|
||||||||||||||||
| for chanName, chanEntry := range doc.SyncData.Channels { | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Avoid compacting Channels entries with Seq==0 SyncData.getCurrentChannels treats both nil removals and non-nil removals with Seq==0 as current channel memberships. CompactDocChannelHistory currently deletes non-nil entries whenever chanEntry.Seq <= seq (seq mode) or when the channel name is listed (channels mode), which can drop a channel that is still considered current and change the effective channel set. Consider skipping deletions for chanEntry.Seq==0 (consistent with GetDocChannelHistory, which only reports removals where Seq != 0), for example:
Suggested change
|
||||||||||||||||
| if chanEntry != nil && chanEntry.Seq <= seq { | ||||||||||||||||
| if chanEntry == nil { | ||||||||||||||||
| continue | ||||||||||||||||
| } | ||||||||||||||||
| var del bool | ||||||||||||||||
| if seq != 0 { | ||||||||||||||||
| del = chanEntry.Seq <= seq | ||||||||||||||||
| } else if len(channels) > 0 { | ||||||||||||||||
| del = slices.Contains(channels, chanName) | ||||||||||||||||
| } | ||||||||||||||||
| if del { | ||||||||||||||||
| compactedChannels.Add(chanName) | ||||||||||||||||
| delete(doc.SyncData.Channels, chanName) | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming the new parameter 'channelsToDelete' might make the multiple references to channel and channels below more readable.