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

tdbPartitionedMatrix will automatically close Array's when done reading #448

Merged
merged 8 commits into from
Jul 17, 2024

Conversation

jparismorgan
Copy link
Collaborator

@jparismorgan jparismorgan commented Jul 16, 2024

What

Today tdbPartitionedMatrix will leave std::unique_ptr<tiledb::Array> partitioned_vectors_array_ and std::unique_ptr<tiledb::Array> partitioned_ids_array_ open until the destructor. This was causing issues in #447 because we have a static function on our indexes which will try to call tiledb::Array::delete_fragments() on these Array's, but that will throw if the Array's are already open somewhere, which they are in tdbPartitionedMatrix.

Testing

  • Existing unit tests pass.
  • Undos changes to Python which were added in Support IVF PQ consolidation by storing raw feature vectors and external IDs #447 to work around this issue.
  • Adds new clear history with an open index unit tests to Vamana and IVFPQ which make sure you can query and then call clear_history(). Note that Vamana doesn't need the changes in this PR to pass that tests, but IVFPQ fails without it.
  • Adds checks for tdb_partioned_matrix.load() = false in several places that didn't have it.

Copy link
Collaborator

@NikolaosPapailiou NikolaosPapailiou left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@jparismorgan jparismorgan merged commit 2e5d929 into main Jul 17, 2024
6 checks passed
@jparismorgan jparismorgan deleted the jparismorgan/tdb-auto-close branch July 17, 2024 08:34
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.

None yet

2 participants