-
Notifications
You must be signed in to change notification settings - Fork 11
Some optimizations for mongo connector #365
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
Conversation
📝 WalkthroughWalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@connectors/mongo/conn.go`:
- Around line 237-243: The goroutine closure in the loop over partitions closes
over the loop variable partition causing races; fix by capturing the current
partition before spawning the goroutine (e.g., assign cur := partition or pass
partition as a parameter) and then use that captured variable inside the eg.Go
closure where ToNS(partition.Namespace) and
c.client.Database(...).Collection(...) are invoked so each goroutine gets the
correct partition value.
🧹 Nitpick comments (1)
internal/app/options/connectorflags.go (1)
757-762: Add a help description for--skip-batch-overwrite.The flag is missing a
Usagefield. Without it, the CLI help output shows an empty description, making the option harder to discover. All other flags in this file includeUsagedescriptions.💡 Suggested update
altsrc.NewBoolFlag(&cli.BoolFlag{ Name: "skip-batch-overwrite", + Usage: "Skip bulk overwrite when duplicate keys are encountered", Destination: &settings.SkipBatchOverwrite, }),
| for _, partition := range partitions { | ||
| if !r.Msg.GetInitialSync() { | ||
| break | ||
| } | ||
| eg.Go(func() error { | ||
| ns, _ := ToNS(partition.Namespace) | ||
| col := c.client.Database(ns.Db).Collection(ns.Col) |
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.
🧩 Analysis chain
🏁 Script executed:
find . -name "conn.go" | grep mongoRepository: adiom-data/dsync
Length of output: 86
🏁 Script executed:
cat -n connectors/mongo/conn.go | sed -n '230,250p'Repository: adiom-data/dsync
Length of output: 723
🏁 Script executed:
cat -n connectors/mongo/conn.go | sed -n '235,260p'Repository: adiom-data/dsync
Length of output: 1010
🏁 Script executed:
rg -n "eg\.Go\(func" connectors/mongo/conn.goRepository: adiom-data/dsync
Length of output: 114
🏁 Script executed:
cat -n connectors/mongo/conn.go | sed -n '1,20p'Repository: adiom-data/dsync
Length of output: 498
🏁 Script executed:
cat -n connectors/mongo/conn.go | sed -n '1060,1085p'Repository: adiom-data/dsync
Length of output: 1103
🏁 Script executed:
cat -n connectors/mongo/conn.go | sed -n '237,270p'Repository: adiom-data/dsync
Length of output: 1617
Capture partition before spawning goroutines.
Line 242 and 254 close over the range variable partition; in Go this variable is reused, so goroutines can observe the last value and race. All concurrent tasks spawned via eg.Go() will reference the same final partition value.
🔧 Safe capture pattern
for _, partition := range partitions {
if !r.Msg.GetInitialSync() {
break
}
+ partition := partition
eg.Go(func() error {
ns, _ := ToNS(partition.Namespace)🤖 Prompt for AI Agents
In `@connectors/mongo/conn.go` around lines 237 - 243, The goroutine closure in
the loop over partitions closes over the loop variable partition causing races;
fix by capturing the current partition before spawning the goroutine (e.g.,
assign cur := partition or pass partition as a parameter) and then use that
captured variable inside the eg.Go closure where ToNS(partition.Namespace) and
c.client.Database(...).Collection(...) are invoked so each goroutine gets the
correct partition value.
Summary by CodeRabbit
New Features
skip-batch-overwriteCLI flag for the MongoDB connector, enabling users to optionally skip bulk overwrite operations during data synchronization.Performance