thinkharderdev commented on a change in pull request #1924:
URL: https://github.com/apache/arrow-datafusion/pull/1924#discussion_r822539024
##########
File path: ballista/rust/scheduler/src/lib.rs
##########
@@ -116,30 +117,29 @@ pub struct SchedulerServer<T: 'static + AsLogicalPlan, U:
'static + AsExecutionP
pub(crate) state: Arc<SchedulerState<T, U>>,
start_time: u128,
policy: TaskSchedulingPolicy,
- scheduler_env: Option<SchedulerEnv>,
+ scheduler_loop: Option<SchedulerLoop>,
executors_client: Option<ExecutorsClient>,
- ctx: Arc<RwLock<ExecutionContext>>,
codec: BallistaCodec<T, U>,
+ /// DataFusion session contexts that are registered within the
SchedulerServer
+ session_context_registry: Arc<SessionContextRegistry>,
Review comment:
So I think we need to preserve some mechanism to register common
extensions in the `Context`. The original intention of adding
`ExecutionContext` to the `SchedulerServer` was to allow creating a
`SchedulerServer` which had a customized `ExecutionContext`. Looks like you've
dealt with custom `ObjectStore` implementations by moving that to `RuntimeEnv`
but there are serveral other extension points which may be useful such as
custom planners, optimizers, udf/udaf
Maybe we can add a "default" `SessionContext` to `SessionContextRegistry` so
we can create new contexts from a template? That way the template context can
be initialized with custom extensions.
##########
File path: ballista/rust/scheduler/src/lib.rs
##########
@@ -1001,10 +1103,56 @@ mod test {
namespace.to_string(),
BallistaCodec::default(),
);
- let ctx = scheduler.ctx.read().await;
- state.init(&ctx).await?;
+ state.init().await?;
// executor should be registered
assert_eq!(state.get_executors_metadata().await.unwrap().len(), 1);
Ok(())
}
}
+
+/// A Registry holds all the datafusion session contexts
+pub struct SessionContextRegistry {
Review comment:
Here maybe we can add `base_context: Option<Arc<SessionContext>>`
--
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]
