Conversation
migrate() and QdrantClient.migrate() both document batch_size as the batch size for scroll and upsert operations, but _migrate_collection never passed it to upload_with_retry, so every write request was sized by upload_points' own default of 64 no matter what the caller asked for. Thread the value through.
✅ Deploy Preview for poetic-froyo-8baba7 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthrough
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
Full details: Linked Issues checkExplanation Issue Full details: Out of Scope Changes checkExplanation The pull request changes
✨ 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 |
1 participant
Problem
migrate()documentsbatch_sizeas covering both reading and writing, but only scrolling ever saw it — every upload was sized byupload_points' own default of 64:qdrant_client/migrate/migrate.py:171—batch_size (int, optional): Batch size for scrolling and uploading vectors. Defaults to 100.QdrantClient.migrate(qdrant_client/qdrant_client.py:2508) —scroll and upsert operations.So
client.migrate(dest, batch_size=1000)still sends 64 points per request. For a large collection that is the difference between a usable migration and hours of round trips, and the documented knob simply does nothing.Cause
_migrate_collection()passesbatch_sizetosource_client.scroll(...)but callsupload_with_retry(...)without it, andupload_with_retry()never forwarded abatch_sizetoclient.upload_points()at all.Change
Thread the value through:
upload_with_retry()gains a trailingbatch_size: int = 64parameter (matchingupload_points' default) and passes it toupload_points(); both call sites in_migrate_collection()forward the caller's value. The parameter is appended to a module-private helper, so existing calls stay compatible, and no public signature changed.Testing
New test in
tests/test_migrate.pyspies onupload_pointsand asserts every write received the requested size:589a87a(test file added, source untouched):1 failed— the spy recordedforwarded [None], i.e. the batch size never arrived.1 passed.Full file at head:
python -m pytest tests/test_migrate.py -q→16 failed, 10 passed, 2 errors; the same command on pristine589a87agives16 failed, 9 passed, 2 errors. Those 16+2 are the pre-existing cases that need a live server (qdrant_client.http.exceptions ... connection refused) and fail identically before and after — the only delta is my new passing test. The new test itself runs fully offline against twoQdrantClient(":memory:")instances.ruff checkpasses on both changed files.ruff format --check --line-length=99reportstests/test_migrate.pywould be reformatted at base and at head with the identical 4-line deviation (thestrict_mode_configblock intest_recreate_collection), so I left that pre-existing drift alone instead of mixing a reformat into this diff —qdrant_client/migrate/migrate.pyis format-clean.Notes for the reviewer
limit=2first scroll in_migrate_collection— that is a separate question, raised once in the closed fix(migrate): first scroll page uses batch_size instead of hard-coded 2 #1217, and unrelated to the upload path.devas the PR template asks; no open PR touchesqdrant_client/migrate/migrate.py(gh pr list --state openfile scan), and fix(migrate): first scroll page uses batch_size instead of hard-coded 2 #1217 was closed unmerged.