Skip to content

Track timely Config changes #295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 12, 2021
Merged

Track timely Config changes #295

merged 2 commits into from
Jan 12, 2021

Conversation

frankmcsherry
Copy link
Member

This PR tracks changes to timely's Configuration type, which is now Config and handles a broader class of options than before. In particular, it houses user-defined configurations, which we are attempting with this PR. Rather than specify DIFFERENTIAL_EAGER_MERGE in the environment, a user can add an isize value with key differential/idle_merge_effort to worker.config() which is then read out by the worker at arrangement construction time. This is a similar idiom to user-specific logging channels, which seems to have worked well enough.

The main downside here is that it was super casual to just screw around with your environment. Now, interested programs must solicit this value somehow, and explicitly set it as part of the configuration. That is more work (and in particular isn't magically done for existing DD programs). This is I suppose the downside of DD not having an execution prelude in the same way that timely has the execute method.

cc: @benesch, @ryzhyk

@benesch
Copy link
Contributor

benesch commented Jan 11, 2021

This seems like a great start to me! One idea I had for a halfway point between differential::execute and this was to add a method like differential::configure that worked like so:

let config = timely::Config::from_args(std::env::args());
differential::config(config, differential::Config { eager_merge: 1000 });
timely::execute(config, ...)

Or via an extension trait:

use differential::Config;
let config = timely::Config::from_args(std::env::args()).differential_eager_merge(1000);
timely::execute(config, ...)

@frankmcsherry frankmcsherry merged commit 1abc8fe into master Jan 12, 2021
This was referenced Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants