Skip to content

Conversation

@gregns1
Copy link
Contributor

@gregns1 gregns1 commented Nov 3, 2025

CBG-4746

  • Changed delay slightly in tool to allow toll to run faster
  • Switch to return list of channel ID not maps from processEntry then deduplicate the channel IDs outside processEntry mutex

Before:
Screenshot 2025-11-03 at 11 09 17

After:
Screenshot 2025-11-03 at 11 08 57

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

  • Link upstream PRs
  • Update Go module dependencies when merged

Integration Tests

Copilot AI review requested due to automatic review settings November 3, 2025 11:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR optimizes channel cache performance by changing the return type of processEntry from a channel set to a slice of channel IDs, and moving channel deduplication outside the mutex-protected section. This reduces contention and improves performance for the cache performance tool.

  • Changed AddToCache and processEntry methods to return []channels.ID instead of channels.Set
  • Updated all callers to convert the slice to a set using channels.SetFromArrayNoValidate for deduplication
  • Reduced delay in cache performance tool from 500ms to 100ms for faster testing

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tools/cache_perf_tool/dcpDataGeneration.go Reduced vBucket creation delay from 500ms to 100ms
db/channel_cache.go Changed AddToCache return type and implementation to use slice instead of set
db/change_cache.go Updated processEntry and related methods to return slices, with deduplication moved outside mutex

Copy link
Collaborator

@torcolvin torcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept seems fine, I think a quick look to see if we can hide the duplication in our our functions is reasonable, and we should definitely document which functions which could return duplicate channels in output.

@gregns1 gregns1 assigned gregns1 and unassigned torcolvin Nov 3, 2025
@gregns1 gregns1 assigned torcolvin and unassigned gregns1 Nov 7, 2025
@gregns1 gregns1 merged commit d469545 into main Nov 7, 2025
58 of 59 checks passed
@gregns1 gregns1 deleted the CBG-4746 branch November 7, 2025 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants