Release the GIL when building indexes

[petehunt]

We're using tantivy-py in an app that creates a lot of Indexes on the fly. During profiling we noticed that it holds the GIL for a few hundred ms each time, making it difficult to use in an asyncio project. This PR releases the GIL for all methods in IndexWriter and Index.

Code was mostly written by Cursor, but I manually reviewed and manually tested and made some manual edits.

Test plan

• nox tests pass
• Upstreamed into our app and deployed to staging. Observed 200ms thread blocks before, observed none after.
• All testcases in our app pass with this change as well
• Manually broke some functions, observed the previous 3 steps fail

[cjrh]

Interesting. Could you add a python test that writes to the same tantivy index from 10 threads in a thread pool? You can use concurrent.futures.ThreadPoolExecutor for example. The assertion can be that in the end the index has all the required documents.

[cjrh]

Oh, and thanks for the submission btw 👍🏼

[cjrh]

Ok, I'm looking into this in more detail. It turns out that allow_threads is now deprecated:

https://docs.rs/pyo3/0.26.0/pyo3/marker/struct.Python.html#method.allow_threads

Instead, they offer detach(). See the docs: https://pyo3.rs/v0.26.0/parallelism.html.

You can likely show Cursor the docs and ask it to update the code for the new detach method.

[cjrh]

This looks like a great addtion thanks. I think we just need to use the new detach() method rather than the now-deprecated allow_threads method.

[petehunt]

Hey thanks for the review. I’ll make these changes. I added the defensive clones manually because I wasn’t sure about the guarantees we lose when releasing the GIL. I’ll add that test, update to the new methods and remove the clones. Will probably get to it late this evening

[petehunt]

Hi @cjrh, I believe I've made all the changes you've requested. As part of the PyO3 upgrade there were a few other deprecations that I fixed.

Additionally, I wrote a concurrency test here: https://gist.github.com/petehunt/eeaabbcf6844c47d601eecfb74e86314, however I couldn't get it to pass with the current release and it doesn't pass in my fork either.

ValueError: Failed to acquire Lockfile: LockBusy. Some("Failed to acquire index lock. If you are using a regular directory, this means there is already an IndexWriter working on this Directory, in this process or in a different process.

I poked around a bit to try to solve it but I'm afraid I'm a little out of my depth on it. Let me know if you need anything else!

[cjrh]

Ok no worries, I'll have a look at the concurrency test.

[cjrh]

@petehunt You might need writer.wait_merging_threads() immediately after the writer.commit() call:

writer = index.writer()

    for doc_num in range(num_docs_per_thread):
        doc = Document()
        doc.add_text("id", f"thread_{thread_id}_doc_{doc_num}")
        doc.add_text("content", f"Content from thread {thread_id}, document {doc_num}")
        doc.add_integer("thread_id", thread_id)
        doc.add_integer("doc_number", doc_num)

        writer.add_document(doc)

    # Commit the changes from this thread
    writer.commit()
    writer.wait_merging_threads()      <-------------- HERE

Well, you definitely need wait_merging_threads(), and it may make the test succeed. Or it may not. I just had a quick glance. I'll look at it more closely tomorrow. The wait_merging_threads() thing is a bit of a gotcha unfortunately. We've recently added a writer context manager that does it for you, but it's not yet in a release.

[cjrh]

Actually, I suspect this LockBusy behaviour is just expected. Presumably one must catch the LockBusy (or ValueError) and retry after a delay or something along those lines. Anyway I'll have a look at it tomorrow. We might merge your PR without the concurrency test and I'll just work on it myself.

[petehunt]

I can take a look at that as well but I suspected it was intended behavior. One thing I tried was releasing the GIL and then acquiring a new per-Index lock, but that didn't resolve the issue either since there were still multiple writers open concurrently.

[cjrh]

I'm going to go ahead and merge. I'll do further testing separately. Thanks again for the contribution 🚀

[petehunt]

Thank you for maintaining the project!