Skip to content

Enable/disable scheduling topology hints through an env. variable - #188

Merged
csegarragonz merged 3 commits into
masterfrom
env-var-topo-hints
Dec 2, 2021
Merged

Enable/disable scheduling topology hints through an env. variable#188
csegarragonz merged 3 commits into
masterfrom
env-var-topo-hints

Conversation

@csegarragonz

@csegarragonz csegarragonz commented Nov 30, 2021

Copy link
Copy Markdown
Collaborator

With this PR scheduling topology hints may be disabled by setting the environment variable: NO_TOPOLOGY_HINTS=on in faam's worker.

@csegarragonz
csegarragonz force-pushed the env-var-topo-hints branch 2 times, most recently from 6be0388 to 4db2985 Compare November 30, 2021 17:45
// If topology hints are disabled, unset the provided topology hint
if (conf.useTopologyHints == "off") {
topologyHint = faabric::util::SchedulingTopologyHint::NORMAL;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change this to check if they're requesting anything other than NORMAL, then print a warning if so. We don't want to have this on accidentally.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Now we:

  1. Check if env var set
  2. If set and hint different than NORMAL, print a warning and unset.

Comment thread src/util/config.cpp Outdated
Comment thread tests/test/util/test_config.cpp
"Test scheduling hints can be disabled through the config",
"[scheduler]")
{
SchedulingConfig config = {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I add the new test

@csegarragonz

Copy link
Copy Markdown
Collaborator Author

I every now and then get errors in the sanitised builds unrelated to the changes introduced. Moving the discussion about this offline.

@csegarragonz
csegarragonz requested review from Shillaker and removed request for Shillaker December 1, 2021 15:27
@csegarragonz csegarragonz self-assigned this Dec 1, 2021
@csegarragonz
csegarragonz merged commit 6ff81aa into master Dec 2, 2021
@csegarragonz
csegarragonz deleted the env-var-topo-hints branch December 2, 2021 09:42
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