Don't pass tiledb context to task graph for vamana ingestion #313
Conversation
Don't pass TileDB context to task graph for vamana ingestion. Context's can't be serialized and sent. Use the passed config instead.
|
This pull request has been linked to Shortcut Story #44663: Don't pass tiledb context to task graph for vamana ingestion. |
| @@ -1602,6 +1601,7 @@ def ingest_vamana( | |||
| # Now that we've ingested the vectors and their IDs, train the index with the data. | |||
| from tiledb.vector_search import _tiledbvspy as vspy | |||
|
|
|||
| ctx = vspy.Ctx(config) | |||
There was a problem hiding this comment.
The code bellow is not performing any taskgraph execution at the moment. @jparismorgan Maybe we need think wether our new Index APIs should receive a config instead of a ctx.
There was a problem hiding this comment.
Interesting, yes, perhaps we need to update to that approach. Specifically is the suggestion to change from index = vspy.IndexVamana(ctx, index_group_uri) to index = vspy.IndexVamana(config, index_group_uri), and then have the index create and hold a reference to the ctx?
@Shelnutt2 the reason I put ctx outside of ingest_vamana is because if I don't then I get the segfault which we just hit in the CI job for this PR: https://github.com/TileDB-Inc/TileDB-Vector-Search/actions/runs/8570643032/job/23489009047?pr=313#step:6:263
There was a problem hiding this comment.
@Shelnutt2 the reason I put ctx outside of ingest_vamana is because if I don't then I get the segfault which we just hit in the CI job for this PR: https://github.com/TileDB-Inc/TileDB-Vector-Search/actions/runs/8570643032/job/23489009047?pr=313#step:6:263
You can't pass a context into a batch task graph because it can't be pickled. We need a solution to that one way or another before we cut a release with vamana. I'm flexible in how you fix it 👍 .
There was a problem hiding this comment.
@Shelnutt2 this was fixed in #347 👍 (should have added you to the review, my bad there)
Don't pass TileDB context to task graph for vamana ingestion. Context's can't be serialized and sent. Use the passed config instead.