feat: use pyo3-asyncio to get a fresh tokio runtime #1950
Closed
+43
−40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PengLiVectra
requested review from
wjones127,
fvaleye,
roeap and
ion-elgreco
as code owners
PengLiVectra
commented
Dec 6, 2023
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| pub fn get_py_storage_backend(&self) -> PyResult<filesystem::DeltaFileSystemHandler> { | ||
| Ok(filesystem::DeltaFileSystemHandler { | ||
| inner: self._table.object_store(), | ||
| rt: Arc::new(rt()?), | ||
| rt: Arc::new(rt_pyo3()?), |
There was a problem hiding this comment.
DeltaFileSystemHandler uses the runtime in utils.rs, which is not pyo3-asyncio. So I keep the original rt but change name to rt_pyo3. Let me know if you have any suggestions.
wjones127
reviewed
Dec 6, 2023
Comment on lines
+56
to
+63
| fn rt_pyo3() -> PyResult<tokio::runtime::Runtime> { | ||
| tokio::runtime::Runtime::new().map_err(|err| PyRuntimeError::new_err(err.to_string())) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn rt() -> &'static tokio::runtime::Runtime { | ||
| pyo3_asyncio::tokio::get_runtime() | ||
| } |
There was a problem hiding this comment.
Why don't we just create one global one in a lazy static instead? I don't think we need the pyo3-asyncio dependency.
|
@PengLiVectra can you add the requested suggestion from Will? |
ion-elgreco
added a commit
that referenced
this pull request
Apr 16, 2024
# Description As suggested by @wjones127 to create a lazy static runtime, supersedes this PR: #1950
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR greatly reduces network connections and dns request volume by the delta-rs library when using Python bindings. The approach here is to utilize pyo3-asyncio's tokio-runtime feature as the source of the Runtime. This yields the same runtime across function calls which preserves connections in the connection pool. The previous code created a new runtime per python function call, which established all new socket connections and issued new DNS requests.
Related Issue(s)
Partly #1315
Testing:
Ran a script that called hundreds of delta operations, and watched tcpdump. Only saw one dns request.