Period task interval config values need to be consistent

[Phil Day]

Currently we have a mix of “==0” and “<=0” being used inside periodic tasks to decide to skip the task altogether. We also have the “spacing=” option in the periodic_task decorator to determine how often to call the task, but in this case: ==0 means “call at default interval” and <0 means “never call”. It would be nice to make these consistent so that all tasks can use the spacing option rather than keep their own check on when (and if) they need to run.

However in order to do this cleanly and not break anyone that is currently using “0 “ to mean “don’t run” we need to:
- Change the default values that are currently 0 to -1
- Log a deprecation warning for the use “*_interval=0”

And then leave this in place until Juno before making the change

[Matthew Gilliard (matthew-gilliard-u)]

So, just to be clear - is it the intention that in all cases:
 - 0 means "use the default polling period"
 - any negative value means "never run this"?

[Phil Day (philip-day)]

>
> So, just to be clear - is it the intention that in all cases:
> - 0 means "use the default polling period"
> - any negative value means "never run this"?
>

I think that should be the eventual goal - but the patch at this stage should be limited to:
- Add test to the help messages of conf values that makes it clear what the current behavior is, and what the future behavior is
- Add warning messages when the config value is 0 is used that the used of 0 for "don’t run" is being deprecated

This is probably worth discussion on openstack-dev

> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1276203
>
> Title:
> Period task interval config values need to be consistent
>
> Status in OpenStack Compute (Nova):
> In Progress
>
> Bug description:
> Currently we have a mix of “==0” and “<=0” being used inside periodic
> tasks to decide to skip the task altogether. We also have the
> “spacing=” option in the periodic_task decorator to determine how
> often to call the task, but in this case: ==0 means “call at default
> interval” and <0 means “never call”. It would be nice to make
> these consistent so that all tasks can use the spacing option rather
> than keep their own check on when (and if) they need to run.
>
> However in order to do this cleanly and not break anyone that is currently
> using “0 “ to mean “don’t run” we need to:
> - Change the default values that are currently 0 to -1
> - Log a deprecation warning for the use “*_interval=0”
>
> And then leave this in place until after Juno before making the change
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/nova/+bug/1276203/+subscriptions

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/72684

[Openstack Gerrit (openstack-gerrit)]

Reviewed: https://review.openstack.org/72684
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=2bfc7171c23d0595aa7f8680271778bc58cb28ba
Submitter: Jenkins
Branch: master

commit 2bfc7171c23d0595aa7f8680271778bc58cb28ba
Author: Matthew Gilliard <email address hidden>
Date: Tue Feb 11 15:28:40 2014 +0000

    Add warning to periodic_task with interval 0

    The behaviour of Oslo's @periodic_task decorator is that
    a repeat interval of 0 means that the method will be
    called regularly. Several Nova methods which use that
    decorator also have code for returning early (ie no-op)
    if the interval is 0. This is inconsistent for users, and
    means there is timing-related code where it doesn't belong.

    This patch adds a warning when a user takes a value of 0
    from config and uses it as an interval in a @periodic_task.

    Ideally all code will use Oslo's convention that 0 means
    run at the default rate and a negative interval will mean
    "don't run". After this warning has been in place for a full
    release cycle that can be the case.

    Change-Id: Ia227f4c4e69ecf361ab02d1d17a3010303650104
    DocImpact: Warns of upcoming change to behaviour when
               *_interval is set to 0
    Closes-Bug: #1272830
    Partial-Bug: #1276203

[Michael Still (mikal)]

We added a warning in Icehouse, so I think its time to finish this change in Juno. Matthew -- are you still working on this or should we unassign it?

[Matthew Gilliard (matthew-gilliard-u)]

Thanks for the reminder Michael. I'll pick this up again.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/107057

[Sean Dague (sdague)]

Added to the rc because this change was intended to be made before the release

[Michael Still (mikal)]

Deferring this change to Kilo.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/107057
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=324d951667aa973955cebd6e49d98af6f2aba916
Submitter: Jenkins
Branch: master

commit 324d951667aa973955cebd6e49d98af6f2aba916
Author: Matthew Gilliard <email address hidden>
Date: Tue Jul 15 14:37:13 2014 +0100

    Remove warning & change @periodic_task behaviour

    During development of Juno, Ia227f4c4e69ecf361ab02d1d17a3010303650104
    added a warning that the behaviour of periodic_tasks with an interval of
    0 would change. This patch changes the behaviour and removes the
    warning.

    Oslo (which provides the @periodic_task decorator) uses spacing=0 to
    mean "run at the default interval of 60s", but several nova methods used
    spacing=0 to mean "don't run". These methods now behave consistently
    with the Oslo code.

    Due to an error in the original patch, some of the warnings refer to the
    intended behaviour change as being in "the K release", and some to "Juno".
    The warning about changing behaviour is first present in Juno, so this
    patch should be merged in Kilo.

    Change-Id: Ie5f2092a3b84e3674fe6a68ce511d2dc218bb625
    Closes-bug: 1276203