Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions connectors/mongo/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ type conn struct {
client *mongo.Client

settings ConnectorSettings
flavor MongoFlavor

nextCursorID atomic.Int64
ctx context.Context
Expand Down Expand Up @@ -265,19 +266,19 @@ func (c *conn) GeneratePlan(ctx context.Context, r *connect.Request[adiomv1.Gene
if numSamples > 1000000 {
slog.Warn("More than 1000000 samples requested", "samples", numSamples)
}
res, err := col.Aggregate(ctx, mongo.Pipeline{{{"$sample", bson.D{{"size", numSamples}}}}, {{"$project", bson.D{{"_id", 1}}}}, {{"$sort", bson.D{{"_id", 1}}}}})
ids, err := c.sampleIDs(ctx, col, numSamples)
if err != nil {
return fmt.Errorf("error getting %v samples for partition: %w", numSamples, err)
}
var factorCount = c.settings.SampleFactor / 2
var low bson.RawValue
for res.Next(ctx) {
for _, id := range ids {
if factorCount > 0 {
factorCount -= 1
continue
}
factorCount = c.settings.SampleFactor - 1
high := res.Current.Lookup("_id")
high := id
ch <- &adiomv1.Partition{
Namespace: partition.GetNamespace(),
EstimatedCount: uint64(c.settings.TargetDocCountPerPartition),
Expand Down Expand Up @@ -566,7 +567,7 @@ func (c *conn) StreamLSN(ctx context.Context, r *connect.Request[adiomv1.StreamL
{{"$match", nsFilter}},
}
}
opts := moptions.ChangeStream().SetStartAfter(bson.Raw(r.Msg.GetCursor()))
opts := c.changeStreamOpts(r.Msg.GetCursor())

changeStream, err := watcher.Watch(ctx, pipeline, opts)
if err != nil {
Expand Down Expand Up @@ -707,7 +708,7 @@ func (c *conn) StreamUpdates(ctx context.Context, r *connect.Request[adiomv1.Str
{{"$match", nsFilter}},
}
}
opts := moptions.ChangeStream().SetStartAfter(bson.Raw(r.Msg.GetCursor())).SetFullDocument("updateLookup")
opts := c.changeStreamOpts(r.Msg.GetCursor()).SetFullDocument("updateLookup")

changeStream, err := watcher.Watch(ctx, pipeline, opts)
if err != nil {
Expand Down Expand Up @@ -995,6 +996,7 @@ func NewConnWithClient(client *mongo.Client, settings ConnectorSettings) adiomv1
return &conn{
client: client,
settings: settings,
flavor: GetMongoFlavor(settings.ConnectionString),
ctx: ctx,
cancel: cancel,
buffers: map[int64]buffer{},
Expand All @@ -1003,6 +1005,16 @@ func NewConnWithClient(client *mongo.Client, settings ConnectorSettings) adiomv1
}
}

func (c *conn) changeStreamOpts(cursor []byte) *moptions.ChangeStreamOptions {
opts := moptions.ChangeStream()
if c.flavor == FlavorDocumentDB {
opts.SetResumeAfter(bson.Raw(cursor))
} else {
opts.SetStartAfter(bson.Raw(cursor))
}
return opts
}
Comment on lines +1008 to +1016

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Check helper implementation and whether cursor guards exist:"
rg -n -C3 'func \(c \*conn\) changeStreamOpts|SetResumeAfter|SetStartAfter|len\(cursor\)' connectors/mongo/conn.go

echo
echo "Check stream call sites using request cursors:"
rg -n -C2 'StreamLSN|StreamUpdates|GetCursor\(\)|changeStreamOpts\(' connectors/mongo/conn.go

echo
echo "Check API definitions/comments for cursor optionality:"
rg -n -C2 'message StreamLSNRequest|message StreamUpdatesRequest|cursor' --iglob '*.proto' || true

Repository: adiom-data/dsync

Length of output: 5660


Add empty cursor guard to changeStreamOpts for consistency.

The function currently passes cursor to SetResumeAfter/SetStartAfter without checking if it's empty. The codebase already uses this pattern in DecodeCursor (line 379), and proto definitions document cursor as optional ("if present"). Add the guard before setting resume options:

Proposed fix
 func (c *conn) changeStreamOpts(cursor []byte) *moptions.ChangeStreamOptions {
 	opts := moptions.ChangeStream()
+	if len(cursor) == 0 {
+		return opts
+	}
 	if c.flavor == FlavorDocumentDB {
 		opts.SetResumeAfter(bson.Raw(cursor))
 	} else {
 		opts.SetStartAfter(bson.Raw(cursor))
 	}
 	return opts
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (c *conn) changeStreamOpts(cursor []byte) *moptions.ChangeStreamOptions {
opts := moptions.ChangeStream()
if c.flavor == FlavorDocumentDB {
opts.SetResumeAfter(bson.Raw(cursor))
} else {
opts.SetStartAfter(bson.Raw(cursor))
}
return opts
}
func (c *conn) changeStreamOpts(cursor []byte) *moptions.ChangeStreamOptions {
opts := moptions.ChangeStream()
if len(cursor) == 0 {
return opts
}
if c.flavor == FlavorDocumentDB {
opts.SetResumeAfter(bson.Raw(cursor))
} else {
opts.SetStartAfter(bson.Raw(cursor))
}
return opts
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@connectors/mongo/conn.go` around lines 1008 - 1016, The changeStreamOpts
function sets resume/start options unconditionally which can pass an empty
cursor; update changeStreamOpts (method on type conn) to guard the resume logic
by checking len(cursor) > 0 before calling opts.SetResumeAfter(bson.Raw(cursor))
or opts.SetStartAfter(bson.Raw(cursor)), mirroring the existing empty-cursor
pattern used in DecodeCursor; only call the Set* method when the cursor bytes
are non-empty so optional cursors are respected.


func maybeUnavailableError(err error) error {
if mongo.IsNetworkError(err) {
return connect.NewError(connect.CodeUnavailable, err)
Expand Down
125 changes: 125 additions & 0 deletions connectors/mongo/docdb.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
package mongo

import (
"bytes"
"cmp"
"encoding/binary"
"encoding/hex"
"fmt"
"sort"

"context"

"go.mongodb.org/mongo-driver/bson"
"go.mongodb.org/mongo-driver/bson/bsontype"
"go.mongodb.org/mongo-driver/mongo"
"golang.org/x/sync/errgroup"
)

var supportedIDTypes = map[bsontype.Type]bool{
bson.TypeObjectID: true,
bson.TypeString: true,
bson.TypeInt32: true,
bson.TypeInt64: true,
bson.TypeBinary: true,
}

func compareBSONRawValues(a, b bson.RawValue) int {
switch a.Type {
case bson.TypeObjectID:
return bytes.Compare(a.Value, b.Value)
case bson.TypeString:
return bytes.Compare(a.Value[4:len(a.Value)-1], b.Value[4:len(b.Value)-1])
case bson.TypeInt32:
ai := int32(binary.LittleEndian.Uint32(a.Value))
bi := int32(binary.LittleEndian.Uint32(b.Value))
return cmp.Compare(ai, bi)
case bson.TypeInt64:
ai := int64(binary.LittleEndian.Uint64(a.Value))
bi := int64(binary.LittleEndian.Uint64(b.Value))
return cmp.Compare(ai, bi)
case bson.TypeBinary:
return bytes.Compare(a.Value[5:], b.Value[5:])
}
panic("compareBSONRawValues called with unsupported type")
}

func (c *conn) sampleIDs(ctx context.Context, col *mongo.Collection, numSamples int64) ([]bson.RawValue, error) {
if c.flavor != FlavorDocumentDB {
res, err := col.Aggregate(ctx, mongo.Pipeline{
{{"$sample", bson.D{{"size", numSamples}}}},
{{"$project", bson.D{{"_id", 1}}}},
{{"$sort", bson.D{{"_id", 1}}}},
})
if err != nil {
return nil, fmt.Errorf("error getting %v samples: %w", numSamples, err)
}
defer res.Close(ctx)
var ids []bson.RawValue
for res.Next(ctx) {
ids = append(ids, res.Current.Lookup("_id"))
}
if res.Err() != nil {
return nil, fmt.Errorf("err iterating through samples: %w", res.Err())
}
return ids, nil
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

type sampleResult struct {
id bson.RawValue
key string
}

results := make([]sampleResult, numSamples)
eg, ctx := errgroup.WithContext(ctx)
for i := int64(0); i < numSamples; i++ {
i := i
eg.Go(func() error {
res, err := col.Aggregate(ctx, mongo.Pipeline{
{{"$sample", bson.D{{"size", 1}}}},
{{"$project", bson.D{{"_id", 1}}}},
})
if err != nil {
return fmt.Errorf("error getting sample %v: %w", i, err)
}
defer res.Close(ctx)
if res.Next(ctx) {
id := res.Current.Lookup("_id")
results[i] = sampleResult{id: id, key: hex.EncodeToString(id.Value)}
}
return nil
})
}
if err := eg.Wait(); err != nil {
return nil, err
}

seen := map[string]struct{}{}
var ids []bson.RawValue
for _, r := range results {
if r.key == "" {
continue
}
if _, dup := seen[r.key]; !dup {
seen[r.key] = struct{}{}
ids = append(ids, r.id)
}
}

if len(ids) > 0 {
t := ids[0].Type
for _, id := range ids[1:] {
if id.Type != t {
return nil, fmt.Errorf("mixed _id types not supported for DocumentDB sampling")
}
}
if !supportedIDTypes[t] {
return nil, fmt.Errorf("unsupported _id type for DocumentDB sampling: %v", t)
}
sort.Slice(ids, func(i, j int) bool {
return compareBSONRawValues(ids[i], ids[j]) < 0
})
}

return ids, nil
}
2 changes: 1 addition & 1 deletion internal/app/options/connectorflags.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ func GetRegisteredConnectors() []RegisteredConnector {
IsConnector: func(s string) bool {
if strings.HasPrefix(s, "mongodb://") || strings.HasPrefix(s, "mongodb+srv://") {
flavor := mongo.GetMongoFlavor(s)
return flavor == mongo.FlavorMongoDB || flavor == mongo.FlavorCosmosDB_VCORE
return flavor == mongo.FlavorMongoDB || flavor == mongo.FlavorCosmosDB_VCORE || flavor == mongo.FlavorDocumentDB
}
return false
},
Expand Down
Loading