Allow 'cylc reload' to reload global configuration #6509

ScottWales · 2024-12-03T03:19:38Z

Add a --global flag to cylc reload that will reload the server's global configuration when set. This can be used to update platform definitions while a workflow is running.

Note that:

Most changes to the [scheduler] section (ports, process pool size, etc.) require a server restart to take effect.
Already installed platforms cannot have their symlink paths changed.
Some changes to platforms (such as changing the hosts) may prevent Cylc from communicating with existing jobs (through polls, kill etc.).

Closes #3762

Check List

I have read CONTRIBUTING.md and added my name as a Code Contributor.
Contains logically grouped changes (else tidy your branch by rebase).
Does not contain off-topic changes (use other PRs for other changes).
Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
Tests are included (or explain why tests are not needed).
Changelog entry included if this is a change that can affect users
Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

oliver-sanders · 2024-12-03T11:56:32Z

Most changes to the [scheduler] section (ports, process pool size, etc.) require a server restart to take effect.

We could do with documenting what can and cannot be reloaded.

The configuration docs are autobuilt from the cfgspec modules so the docs come in this PR rather than a cylc-doc one. We used the RestructuredText markup, could use a .. note:: Reloading directive or something of the ilk.

cylc-flow/cylc/flow/cfgspec/globalcfg.py

Lines 739 to 741 in 9ff50f8

    
           with Conf('scheduler', desc=( 
        
               default_for(SCHEDULER_DESCR, "[scheduler]", section=True) 
        
           )):

We define some common messages as module constants and template them in as needed, e.g:

cylc-flow/cylc/flow/cfgspec/globalcfg.py

Lines 341 to 347 in 9ff50f8

    
           MAIL_DESCR = ''' 
        
           Settings for the scheduler to send event emails. 
        
           These settings are used for both workflow and task events. 
        
           .. versionadded:: 8.0.0 
        
           '''

Some changes to platforms (such as changing the hosts) may prevent Cylc from communicating with existing jobs (through polls, kill etc.).

That sounds pretty bad! Presumably caused by platform definitions lingering from the original config? Or is it more subtle?

cylc/flow/task_proxy.py

ScottWales · 2024-12-03T23:09:23Z

Some changes to platforms (such as changing the hosts) may prevent Cylc from communicating with existing jobs (through polls, kill etc.).

That sounds pretty bad! Presumably caused by platform definitions lingering from the original config? Or is it more subtle?

Say a job starts with platform definition

[platforms]
    [[foo]]
        hosts = a.example.com
        job runner = background

While the job is running, the global configuration is changed to point to a different host

[platforms]
    [[foo]]
        hosts = b.example.com
        job runner = background

The existing job will continue running, and once it finishes it should report back to the Cylc server fine, however if the Cylc server tries to poll or kill the task any messages will get sent to the new platform host b.example.com and not find the job. Changing the job runner may also break the server communicating with jobs running during the reload.

I don't think doing this should be blocked, it's one of the original use cases in #3762, but its worth highlighting in documentation.

oliver-sanders · 2024-12-04T13:28:16Z

If a.example.com and b.example.com are both hosts belong to the same platform, we should be able to change the hosts in the way you outline above without error. E.G, admins might want to add or remove login nodes from the platform for whatever reason.

If these hosts belong to two different platforms with two discrete queues, then failure is correct. This isn't a reasonable global config change, they are replacing one platform with another but using the same name for both, errors are expected.

oliver-sanders · 2024-12-05T14:17:07Z

How does this functionality play with environment variables defined in the rose-suite.conf file?

See #5418 (comment)

ScottWales · 2024-12-05T23:39:52Z

Any environment variables that the cylc-rose plugin sets is kept constant through the reload

ScottWales · 2024-12-09T00:11:07Z

It's not clear to me why the functional test is failing. Could be a timing thing, the workflow file is trying to do a cylc pause but that fails as it's using the wrong variable name, should be CYLC_WORKFLOW_ID I think.

cylc-flow/tests/functional/cylc-combination-scripts/vr_workflow/flow.cylc

Lines 6 to 8 in e3f77a5

    
           [runtime] 
        
               [[foo]] 
        
                   script = cylc pause ${WORKFLOW_ID}

oliver-sanders · 2024-12-10T09:54:11Z

Yes, that environment variable doesn't look right, but how was the test passing before?

@wxtim, could you take a look at that functional test when you get a chance.

wxtim · 2024-12-10T13:22:44Z

Yes, that environment variable doesn't look right, but how was the test passing before?

That test is dubious (but probably not fatal since it's testing reload behaviour), but well spotted. I'll fix it on master.

However, it is failing correctly in this case:

I've opened a PR against your PR which should fix the problem and explains a little more: ScottWales#11

by both Cylc VR and Reload. Mirrors practice for VALIDATE, INSTALL, PLAY and RESTART

Add a store for CLI variables for reload, which can then be accessed

ScottWales · 2024-12-11T05:20:28Z

Thanks Tim, that's fixed the tests

oliver-sanders · 2024-12-11T09:58:50Z

Reload has been a notoriously difficult feature to maintain and a major source of bugs, so we need to flesh out exactly what does and does not get reloaded and ensure we pin down our desired behavior with tests.

Result of a quick skim of the global config (recommend a more detailed inspection):

[scheduler] - documented as not reloadable, good.
- [scheduler][events] - From code inspection, this will probably get reloaded contrary to the documentation? Needs testing.
- [scheduler][mail] - From code inspection, this will probably get reloaded contrary to the documentation? Needs testing.
[install][symlink dirs] - documented as not reloadable, good.
[platforms] - tested platform metadata gets reloaded in an integration test, but haven't tested that jobs get submitted with the new platform config post-reload. This is an important use case and job submission is fiddly, so we could do with a formal test to ensure this works now and doesn't get broken in the future.
[platform groups] - Not tested but should behave the same as platforms. Recommend at least extending the integration test to include a [platform groups] example to ensure there is some coverage of this feature.
[task events] - From code inspection, this will probably get reloaded contrary to the documentation? Needs testing.

ScottWales · 2024-12-12T05:38:28Z

For [scheduler], it might be easier if glbl_cfg(reload=True) just doesn't update that section? That would avoid a mix of settings that are and aren't reloadable, e.g. right now [scheduler][mail]task event batch interval is cached by the Scheduler so not reloadable but [scheduler][mail]from is not. Although that's the case currently for the workflow configuration as well which these act as defaults for. I'll update the docs to mark [scheduler][mail] and [scheduler][events] as special cases.

Added a functional test for reload --global that shows [platform] and [task events] being updated. I don't think [task events] is documented as not being reloadable?

Added a integration test for platform group updates.

oliver-sanders · 2025-01-28T14:50:24Z

cylc/flow/network/schema.py

+        reload_global = Boolean(
+            default_value=False,
+            description="Also reload global config")
+


FYI, all arguments added here are automatically listed in the GUI. The description text goes into the help bubble.

Here's how this option will appear:

oliver-sanders · 2025-01-28T15:12:53Z

cylc/flow/commands.py

@@ -361,6 +362,25 @@ async def reload_workflow(schd: 'Scheduler'):
        # give commands time to complete
        sleep(1)  # give any remove-init's time to complete

+    if reload_global:
+        # Reload global config if requested
+        LOG.info("Reloading the global configuration.")


This will update the workflow status message (shown in the GUI toolbar) to reflect what Cylc is doing:

Suggested change

LOG.info("Reloading the global configuration.")

LOG.info("Reloading the global configuration.")

schd.reload_pending = 'reloading the global configuration'

schd.update_data_store() # update workflow status msg

await schd.update_data_structure()

It's useful info incase the operation gets stuck with a filesystem issue, or incase loading the config takes a long time for whatever reason.

oliver-sanders · 2025-01-28T15:28:47Z

cylc/flow/commands.py

+                ' workflow.'
+                '\nOtherwise, fix the configuration and attempt to reload'
+                ' again.'
+            )


The way this is implemented, if the global config fails to reload, we log the error, but continue and reload the workflow config.

I see cylc reload --global as a transaction that reloads both the global config and the workflow config. Ideally, if either of these operations fail, we would rollback the transaction. I.e, either both operations happen or neither happen, the workflow is always left in an intended state.

So I would expect it to bail at this point. I.E, If the global reload fails, I would expect the logic to skip straight to this block of logic at the end of the routine:

# resume the workflow if previously paused schd.reload_pending = False schd.update_data_store() # update workflow status msg schd._update_workflow_state() if not was_paused_before_reload: schd.resume_workflow() schd.process_workflow_db_queue() # see #5593

That does leave us with the situation where the global config does reload, but the workflow config doesn't (e.g. a validation error). Currently, the global config change will not roll back in this situation, but we could get it to do that something along the lines of:

if reload_global: ... try: cfg = glbl_cfg(cached=False) except ...: ... ... # reload the workflow definition ... glbl_cfg().reload(cfg)

(would require a reload method on the GlobalConfig object)

oliver-sanders · 2025-01-28T15:29:06Z

changes.d/6509.feat.md

@@ -0,0 +1 @@
+Added --global flag to 'cylc reload'


Suggested change

Added --global flag to 'cylc reload'

Added --global flag to 'cylc reload' which also reloads the Cylc global config.

oliver-sanders · 2025-01-28T15:38:56Z

cylc/flow/cfgspec/globalcfg.py

+
+   The majority of scheduler settings affect the server and cannot be reloaded
+   with ``cylc reload --global``, the server must be stopped and restarted for
+   changes to take effect except for the sections [mail] and [events] which


Little bit of Cylc Sphinx magic, use single backquotes (or the log role :cylc:conf) to reference configuration sections:

Suggested change

changes to take effect except for the sections [mail] and [events] which

changes to take effect except for the sections `[mail]` and `[events]` which

This generates links, but also runs the references through the internal consistency checker to make sure they stay valid.

oliver-sanders · 2025-01-28T15:40:01Z

cylc/flow/cfgspec/globalcfg.py

@@ -108,6 +108,13 @@

   Not to be confused with :cylc:conf:`flow.cylc[scheduling]`.

+.. note::


FYI: This note will only appear in the workflow config reference, not the global config reference due to the default_for function which only includes the first paragraph.

oliver-sanders · 2025-01-28T15:59:11Z

cylc/flow/network/schema.py

@@ -1919,6 +1919,10 @@ class Meta:
    class Arguments:
        workflows = graphene.List(WorkflowID, required=True)

+        reload_global = Boolean(


Unfortunately, this is a breaking change that will cause errors when cylc reload is run against workflows using a different version both on the CLI and in the GUI.

Somewhat surprisingly, we don't yet have a working pattern for adding new fields to mutations. It's one of the things on the TODO list that we haven't found time for yet!

If the new field appears in the mutation and the scheduler doesn't know about it -> error.

If the new field doesn't appear in the mutation but the scheduler is expecting it -> error.

I think we can get around both issues by marking this field as optional (required=False) and omitting the new field from the cylc reload mutation unless the --global option is used.

This way the mutation should remain forward and backward compatible so long as the --global option isn't used. And should only fail for older schedulers where the --global option is used (which is correct).

Sadly, we don't have any automated inter-version testing as yet (another one for the TODO list) so this will need to be tested manually for now.

oliver-sanders · 2025-01-28T16:08:09Z

cylc/flow/scripts/reload.py

 ) {
  reload (
    workflows: $wFlows
+    reloadGlobal: $reloadGlobal


[In relation to above comment]

I think we can avoid inter-version interoperability issues providing we omit the new reloadGlobal field when --global was not specified.

ScottWales added 3 commits December 3, 2024 14:21

Reload global platforms

3e9e9a6

Add flag to command

656c275

Test errors are handled

2ae8bb3

ScottWales force-pushed the reload-global branch from 44554e6 to 2ae8bb3 Compare December 3, 2024 03:22

ScottWales added 2 commits December 3, 2024 14:31

Fix lint and tests

9b7c1e8

Update test structure

7f3bd41

oliver-sanders assigned ScottWales Dec 3, 2024

oliver-sanders added this to the 8.5.0 milestone Dec 3, 2024

oliver-sanders reviewed Dec 3, 2024

View reviewed changes

cylc/flow/task_proxy.py Outdated Show resolved Hide resolved

ScottWales added 4 commits December 5, 2024 15:07

Revert proxy change, fix test

7be6be5

Add notes for settings not reloadable

d6fd822

Add changelog entry

172537d

Merge remote-tracking branch 'origin/master' into reload-global

2236a70

hjoliver mentioned this pull request Dec 5, 2024

2024 meetings cylc/cylc-admin#188

Open

wxtim and others added 3 commits December 10, 2024 14:12

Add a store for CLI variables for reload, which can then be accessed

49bf3de

by both Cylc VR and Reload. Mirrors practice for VALIDATE, INSTALL, PLAY and RESTART

Merge pull request #11 from wxtim/fix.for_Scott_Wales

257e817

Add a store for CLI variables for reload, which can then be accessed

Add coverage over verbose

557f74d

ScottWales added 3 commits December 12, 2024 16:12

Add global reload functional test

b4542e1

Test reloading platform groups

7f5bfb1

Update scheduler reload docs

8c93385

hjoliver mentioned this pull request Jan 27, 2025

2025 meeting notes cylc/cylc-admin#200

Open

oliver-sanders reviewed Jan 28, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow 'cylc reload' to reload global configuration #6509

Allow 'cylc reload' to reload global configuration #6509

ScottWales commented Dec 3, 2024 •

edited by oliver-sanders

Loading

oliver-sanders commented Dec 3, 2024 •

edited

Loading

ScottWales commented Dec 3, 2024

oliver-sanders commented Dec 4, 2024 •

edited

Loading

oliver-sanders commented Dec 5, 2024

ScottWales commented Dec 5, 2024

ScottWales commented Dec 9, 2024

oliver-sanders commented Dec 10, 2024

wxtim commented Dec 10, 2024 •

edited

Loading

ScottWales commented Dec 11, 2024

oliver-sanders commented Dec 11, 2024

ScottWales commented Dec 12, 2024

oliver-sanders Jan 28, 2025

oliver-sanders Jan 28, 2025

oliver-sanders Jan 28, 2025

oliver-sanders Jan 28, 2025

oliver-sanders Jan 28, 2025

oliver-sanders Jan 28, 2025

oliver-sanders Jan 28, 2025

oliver-sanders Jan 28, 2025

-        LOG.info("Reloading the global configuration.")
+        LOG.info("Reloading the global configuration.")
+        schd.reload_pending = 'reloading the global configuration'
+        schd.update_data_store()  # update workflow status msg
+        await schd.update_data_structure()

	Added --global flag to 'cylc reload'
	Added --global flag to 'cylc reload' which also reloads the Cylc global config.

	changes to take effect except for the sections [mail] and [events] which
	changes to take effect except for the sections `[mail]` and `[events]` which

		@@ -108,6 +108,13 @@

		Not to be confused with :cylc:conf:`flow.cylc[scheduling]`.

		.. note::

Allow 'cylc reload' to reload global configuration #6509

Are you sure you want to change the base?

Allow 'cylc reload' to reload global configuration #6509

Conversation

ScottWales commented Dec 3, 2024 • edited by oliver-sanders Loading

oliver-sanders commented Dec 3, 2024 • edited Loading

ScottWales commented Dec 3, 2024

oliver-sanders commented Dec 4, 2024 • edited Loading

oliver-sanders commented Dec 5, 2024

ScottWales commented Dec 5, 2024

ScottWales commented Dec 9, 2024

oliver-sanders commented Dec 10, 2024

wxtim commented Dec 10, 2024 • edited Loading

ScottWales commented Dec 11, 2024

oliver-sanders commented Dec 11, 2024

ScottWales commented Dec 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ScottWales commented Dec 3, 2024 •

edited by oliver-sanders

Loading

oliver-sanders commented Dec 3, 2024 •

edited

Loading

oliver-sanders commented Dec 4, 2024 •

edited

Loading

wxtim commented Dec 10, 2024 •

edited

Loading