DataFusion Configuration Consolidation

[alamb]

Related

apache/datafusion-ballista#479
#3885

TLDR Recommendations

This is a complicated issue and I don't have a magic answer. However I have some concrete suggestions

Some suggested steps:

• Config Cleanup: Remove TaskProperties and KV structure, keep key=value serialization  #4382
• We should consolidate ExecutionProps and TaskProperties also.
• Allow configuring parquet filter pushdown dynamically  #3821
• Consolidate SessionConfig and ConfigOptions #3887
• Make ConfigOptions easier to work with #3886
• Make ConfigOption names into an Enum  #4517

I think consolidating SessionConfig/Config options is likely to be the most controversial / cause the most chrun but it will provide immense benefits I think (like runtime visibility into the current settings)

Then we can further improve from there

Introduction

"Configuration" in DataFusion has a few usecases:

• set a values from a string provided by the user (set XX = YY in datafusion-cli)
• set a value from environment variables -- See ConfigOptions::from_env
• display as a string (e.g. SHOW in datafusion-cli`)
• Documented in the datafusion docs: https://arrow.apache.org/datafusion/user-guide/configs.html
• serialize / deserialize over the network (e.g. Ballista)
• settable programmatically (e.g. via the dataframe API)

There are also two overlapping "levels" of configuration that are needed

• Session level (e.g. that can be reused from one query execution to the next)
• Statement/Task level (e.g. that is needed to plan a query and doesn't change for the duration of a statement such as "the value of now()" and target batch sizes, etc). Statement level configuration is typically a superset of the session level configuration

Current state of configuration in DataFusion

The current state is .... inconsistent to put it mildly.

The core structure is SessionContext which is the final glue and entry point to interacting with datafusion (e.g. tables provided, etc).

Within the SessionContext there is the some combination of SessionState, SessionConfig, ConfigOptions. Part of the hierarchy is like this:

SessionContext
 -- Has a SessionState
 -- SessionStartTime
 -- SessionConfig
   -- ConfigOptions

SessionConfig is effectively the Session level configuration I describe above.

TaskContext is the statement level (aka per task / per query) level context. If you look hard you can see has a copy of the SessionConfig (buried in TaskPropertoes) or also maybe is backed by KVPairs.

Desire

I would like to have a clear configuration system that cleanly separates the statement level config from the task level config and allows configuration values to be set in a uniform manner and that are easy to view programmatically

[alamb]

However, one question, raised by @avantgardnerio in apache/datafusion-ballista#479, is how to structure the interactions with the configuration code.

It turns out that SessionConfig has a more rust struct style and ConfigOptions is a more dynamic hashmap style. I honestly don't have a preference as long as the introspection (with docstrings) features of ConfigOptions are preserved. Maybe this is possible via macro magic

[alamb]

🤔 there is also ExecutionProps which is some subset of the TaskProperties 🤔

[mingmwang]

I think consolidating SessionConfig/Config options is likely to be the most controversial / cause the most chrun but it will provide immense benefits I think (like runtime visibility into the current settings)

I think consolidating SessionConfig/ConfigOptions is definitely the right direction. The debate should be how to consolidate them, either keep SessionConfig or keep ConfigOptions. I do not have a clear preference on this either. maybe we can have a vote.

[mingmwang]

🤔 there is also ExecutionProps which is some subset of the TaskProperties 🤔

We should consolidate ExecutionProps and TaskProperties also.

[mingmwang]

I'm OK with the change in this PR. Ballista should still work after this PR.

#4382

[alamb]

Here is my next contribution to clean up configuration: #4427 (slowly consolidating the configurations)

[Cheappie]

If a TaskContext represents a query/task, then could TableProvider receive in scan method TaskContext as replacement for SessionState ? I would like to have a way to identify task in TableProvider scan fn. If I am not wrong, the only way right now to identify task in TableProvider is to create new SessionContext for each query and use session_id from SessionState.

[tustvold]

If a TaskContext represents a query/task

I think it might be more precise to describe TaskContext as the state for query execution, with SessionState used for query planning.

the only way right now to identify task in TableProvider

Correct, at the point in planning where TableProvider::scan is called, the task conceptually doesn't exist yet

create new SessionContext for each query

FWIW this is what IOx does

Edit: I wrote some docs in #2655 last time I got thoroughly confused by this system that I think are still in date. I see we have grown a few more config structs since then though

Edit Edit: It may also be the case that a single query is broken into multiple TaskContext - I think Ballista does this, but I'm not sure.

[Cheappie]

Thank you @tustvold for clarification, in such case I will follow SessionContext per query approach.

[alamb]

I think this is largely complete and we don't have any additional work planned here, so closing