andygrove opened a new issue, #2533: URL: https://github.com/apache/datafusion-ballista/issues/2533
**Describe the bug** Changing S3 settings with `SET s3.*` partway through a session does not reach executors that have already run a task for that session. They keep building S3 stores from the values the session's first task carried. The cause is the interaction between two pieces: - `runtime_env_with_s3_support` builds a `CustomObjectStoreRegistry` that holds a clone of the `S3Options` from the `SessionConfig` it was called with ([object_store.rs](https://github.com/apache/datafusion-ballista/blob/e25c2a55d70d9692c2982c1d07db90f5b9f61897/ballista/core/src/object_store.rs#L65-L83)). - The executor builds a fresh `SessionConfig` for every task (`produce_config()` then `update_from_key_value_pair(&task.props)`), but `DefaultSessionRuntimeCache` returns the base `RuntimeEnv` cached under the session id, and only calls the base producer on a miss ([runtime_cache.rs](https://github.com/apache/datafusion-ballista/blob/e25c2a55d70d9692c2982c1d07db90f5b9f61897/ballista/executor/src/runtime_cache.rs#L114-L138)). So the registry inside the cached runtime still points at the first task's `S3Options`, and the new task's options are never read. This applies to every `s3.*` key: `access_key_id`, `secret_access_key`, `session_token`, `region`, `endpoint` and `allow_http`. The scheduler is not affected, since `create_or_update_session` builds a new context from the submitted config each time. That means planning uses the new values while execution may use the old ones. It also varies by executor: one that has not seen the session yet, or has evicted it from the LRU (`--session-runtime-cache-capacity`, default 16), picks up the new values. The practical case is temporary credentials. A client that refreshes an expiring `s3.session_token` with another `SET` has no effect on executors that already cached the session. The session cache came in with #1995. **To Reproduce** This test mirrors what `execution_loop` does per task. It passes today, showing the second task in session `s1` still gets a store built with `KEY_ONE`: ```rust fn task_config(key_id: &str) -> SessionConfig { session_config_with_s3_support().update_from_key_value_pair(&[ kv("s3.access_key_id", key_id), kv("s3.secret_access_key", "secret"), kv("s3.region", "us-east-1"), ]) } #[test] fn second_task_keeps_first_tasks_s3_options() { let url = ObjectStoreUrl::parse("s3://bucket").unwrap(); let cache = DefaultSessionRuntimeCache::new( Arc::new(runtime_env_with_s3_support), Arc::new(|base, _, _| Ok(base)), 16, ); let rt1 = cache.produce_runtime("s1", &task_config("KEY_ONE"), 1).unwrap(); rt1.object_store(&url).unwrap(); let rt2 = cache.produce_runtime("s1", &task_config("KEY_TWO"), 1).unwrap(); let store = format!("{:?}", rt2.object_store(&url).unwrap()); assert!(store.contains("KEY_ONE")); assert!(!store.contains("KEY_TWO")); } ``` A different session id with the same config gets `KEY_TWO`, as expected. **Expected behavior** A task uses the S3 settings it was launched with. **Additional context** Some possible directions: - Include the config the base producer depends on in the cache key, for example a hash of the session's extension entries, so a changed config builds a new base runtime. - Stop capturing `S3Options` in the registry, and resolve them per task instead. Found while working on #2532, which would add more session-level S3 options that hit the same problem. -- 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]
