Skip to content

Conversation

rtyler
Copy link
Member

@rtyler rtyler commented Jul 24, 2025

FREEZE!

Looks like we were really close to full multi-threaded support before,
and just needed to sprinkle a little macro magic on the pyclass
definition.

See pyo3 docs

Fixes #3594

@github-actions github-actions bot added the binding/python Issues for the Python package label Jul 24, 2025
@rtyler rtyler marked this pull request as ready for review July 24, 2025 05:42
@rtyler rtyler requested a review from ion-elgreco as a code owner July 24, 2025 05:42
@rtyler rtyler enabled auto-merge July 24, 2025 05:43
Copy link

codecov bot commented Jul 24, 2025

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.50%. Comparing base (765e129) to head (d41465c).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
python/src/lib.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3618      +/-   ##
==========================================
- Coverage   74.53%   74.50%   -0.03%     
==========================================
  Files         146      146              
  Lines       44919    44919              
  Branches    44919    44919              
==========================================
- Hits        33480    33467      -13     
- Misses       9247     9251       +4     
- Partials     2192     2201       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ion-elgreco
Copy link
Collaborator

@rtyler nice stuff, didn't know about Frozen but makes total sense. The struct properties are only updated within rust and not python

I don't think Polars allows the DataFrame to be references across a threadpoolexecutor, only processpoolexecutor with spawn

@rtyler
Copy link
Member Author

rtyler commented Jul 24, 2025

I don't think Polars allows the DataFrame to be references across a threadpoolexecutor, only processpoolexecutor with spawn

There is no cross thread dataframe access here, this is entirely about our DeltaTable

:😕

@ion-elgreco
Copy link
Collaborator

I don't think Polars allows the DataFrame to be references across a threadpoolexecutor, only processpoolexecutor with spawn

There is no cross thread dataframe access here, this is entirely about our DeltaTable

:😕

The multi thread test is using a Polars Dataframe, but it causes this error:

       return self._df.__arrow_c_stream__(requested_schema)
E       RuntimeError: Already borrowed

.venv/lib/python3.9/site-packages/polars/dataframe/frame.py:1466: RuntimeError

@rtyler
Copy link
Member Author

rtyler commented Jul 24, 2025

I don't know why that was failing in CI, it didn't fail for me locally. A new run also hasn't failed 🤔 🤷

@rtyler rtyler force-pushed the fix/multi-threaded-again-3594 branch from e6ecd74 to 0e559a0 Compare July 24, 2025 15:02
FREEZE!

Looks like we were _really_ close to full multi-threaded support before,
and just needed to sprinkle a little macro magic on the pyclass
definition.

See [pyo3 docs](https://pyo3.rs/v0.23.0/class/thread-safety)

Fixes delta-io#3594

Signed-off-by: R. Tyler Croy <[email protected]>
@rtyler rtyler force-pushed the fix/multi-threaded-again-3594 branch from 0e559a0 to d41465c Compare July 24, 2025 16:08
@rtyler rtyler added this pull request to the merge queue Jul 24, 2025
Merged via the queue into delta-io:main with commit c8c304a Jul 24, 2025
28 of 29 checks passed
@rtyler rtyler deleted the fix/multi-threaded-again-3594 branch July 24, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/python Issues for the Python package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python DeltaTable does not support writes in multiple threads (again?)
2 participants