viirya commented on code in PR #120:
URL:
https://github.com/apache/spark-connect-rust/pull/120#discussion_r4421063398
##########
crates/spark-connect/src/dataframe.rs:
##########
@@ -769,14 +821,48 @@ impl DataFrame {
}
/// Get the schema of this DataFrame.
+ ///
+ /// The first successful lookup is cached and shared with exact clones.
+ /// Selected schema-preserving transformations snapshot an already resolved
+ /// schema; set operations do so only when both cached input schemas match
+ /// exactly in the same session. Declared struct output schemas also seed
+ /// the cache. Other plans resolve independently.
+ /// Concurrent lookups on the same DataFrame or its clones wait for a
single
+ /// analysis RPC, so a stalled analysis blocks those metadata readers.
+ /// Changes to external table or view metadata do not invalidate this
cache;
+ /// recreate the DataFrame from its source to resolve the schema again.
+ /// Each call returns an independent value, and failed lookups can be
retried.
pub fn schema(&self) -> Result<DataType> {
+ // Keep the lock during analysis so concurrent accesses issue only one
+ // successful RPC. Errors leave the cache empty and can be retried.
+ let mut cached = self
+ .schema_cache
+ .lock()
+ // Unlike the other cache locks, this guard spans a blocking RPC.
+ // Recover defensively from Rust callers that panic during analysis
+ // (for example, by calling the sync API inside a Tokio runtime).
+ // The slot contains only None or a complete schema, so it is safe.
+ .unwrap_or_else(|poisoned| {
+ self.schema_cache.clear_poison();
+ poisoned.into_inner()
+ });
+ if let Some(schema) = cached.as_ref() {
+ return Ok(schema.as_ref().clone());
Review Comment:
Small one: the deep clone here happens while the guard is held. A concurrent
`with_cached_schema` on another thread then fails `try_lock` and builds a child
with an empty cache. Something like this keeps the critical section to a
refcount bump:
```rust
if let Some(schema) = cached.clone() {
drop(cached);
return Ok(schema.as_ref().clone());
}
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
