shyjsarah commented on code in PR #873:
URL: https://github.com/apache/paimon-rust/pull/873#discussion_r4055941025
##########
crates/integrations/datafusion/src/runtime.rs:
##########
@@ -103,14 +103,18 @@ where
F: Future + Send + 'static,
F::Output: Send + 'static,
{
- if Handle::try_current().is_ok() {
- let handle = global_runtime().handle().clone();
- std::thread::spawn(move || handle.block_on(future))
+ if let Ok(handle) = Handle::try_current() {
+ if handle.runtime_flavor() == RuntimeFlavor::MultiThread {
Review Comment:
Thanks, this is correct. `runtime_flavor()` describes the runtime
implementation, but not whether the current execution context permits
`block_in_place`. A `LocalSet` changes the latter, so #873 introduces the
regression you described and should not be merged as-is.
I reworked the fix in #881. Instead of trying to detect when in-place
blocking is safe, it removes the sync-to-async bridge from the affected catalog
discovery callbacks: `schema_names`, `schema`, `table_names`, and
`table_exist`. Metadata is loaded or refreshed at async boundaries, while those
synchronous callbacks only read an in-memory snapshot.
This avoids both the worker-starvation issue from #872 and the `LocalSet`
panic. #881 also tests the stronger invariant that synchronous discovery
callbacks do not access the remote catalog, and preserves the last good
snapshot when refresh fails.
I propose superseding and closing #873 in favor of #881. Thanks for catching
this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
