Skip to content
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

upload_records() calls in _migrate_collection() should use wait=True #403

Closed
andrew-titus opened this issue Dec 12, 2023 · 2 comments
Closed

Comments

@andrew-titus
Copy link

Since the calls to upload_records (here and here) in _migrate_collection() do not use wait=True (which defaults to False), the assertion statement at the end of the function that checks that the vector counts match often fails when migrating content from a local client to a remote client. Since we are checking that they match, we should wait for any uploads to complete for consistency, so this should either be set to wait=True directly or at least be exposed as a kwarg for users. Thanks!

@shivas1516
Copy link
Contributor

shivas1516 commented Jan 1, 2024

I would like to take on this issue. After reviewing the code, I understand that the calls to upload_records in _migrate_collection() may not wait for uploads to complete, potentially leading to inconsistencies in vector counts. I plan to address this.
If there are any specific considerations or preferences regarding the solution, please let me know. I'll keep you updated on my progress.

shivas1516 added a commit to shivas1516/qdrant-client that referenced this issue Jan 8, 2024
…during client migration. The problem was due to missing in the calls. This commit adds to ensure proper synchronization and resolves the vector count mismatch.
joein added a commit that referenced this issue Jan 11, 2024
* Addresses issue #403 where the vector counts were inconsistent during client migration. The problem was due to missing  in the  calls. This commit adds  to ensure proper synchronization and resolves the vector count mismatch.

* Update qdrant_client/migrate/migrate.py

* Update qdrant_client/migrate/migrate.py

---------

Co-authored-by: George <[email protected]>
@joein
Copy link
Member

joein commented Jan 11, 2024

Thanks for highlighting this issue!

I believe the main issue with upload_records in the loop had already been resolved by @agourlay

The remaining part of it with a corner case should be fixed with #428 and available with the next release, thanks @shivas1516

Feel free to re-open the issue if the problems is reproducible in qdrant-client 1.7.1 (should be available this week)

@joein joein closed this as completed Jan 11, 2024
joein added a commit that referenced this issue Jan 19, 2024
* Addresses issue #403 where the vector counts were inconsistent during client migration. The problem was due to missing  in the  calls. This commit adds  to ensure proper synchronization and resolves the vector count mismatch.

* Update qdrant_client/migrate/migrate.py

* Update qdrant_client/migrate/migrate.py

---------

Co-authored-by: George <[email protected]>
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

No branches or pull requests

3 participants