CINDER_PERIODIC_INTERVAL is unnecessary and perhaps dangerous

[Brian Rosmaita]

Change I20e52e66fcc94b224476cdd14c88bd6981b4e617 introduced a devstack local.conf var CINDER_PERIODIC_INTERVAL [0] as a workaround for Bug #1180976. The fix for that bug [1] does not require adjustment of cinder's periodic_interval config option.

CINDER_PERIODIC_INTERVAL isn't currently used anywhere in devstack except in lib/cinder.

Need to verify that it's not used in the setup code for dsvm test jobs for tempest, and then remove it from devstack/lib/cinder.

Question: the devstack default for the value is 60 (which is the same as the cinder default), so why not just leave it?
Answer: periodic_interval is used in multiple places in cinder, and adjusting its value to tune one function may have adverse effects on other functionality. See Bug #1823748.

[0] https://github.com/openstack-dev/devstack/commit/f35ff72b77d479d43c1ede6b9f691ae54a2c60a1
[1] https://github.com/openstack/cinder/commit/642526013c855e88873ecb96e8680698e095d83a

[Brian Rosmaita (brian-rosmaita)]

Using 'grep -R -I -n -i CINDER_PERIODIC_INTERVAL *', I don't find any use of the var in the openstack-infra/openstack-zuul-jobs repo.

[OpenStack Infra (hudson-openstack)]

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

[Brian Rosmaita (brian-rosmaita)]

Jens Harbott found some uses via codesearch.openstack.org:

http://git.openstack.org/cgit/openstack-dev/devstack/tree/.zuul.yaml#n383
http://git.openstack.org/cgit/openstack-infra/devstack-gate/tree/devstack-vm-gate.sh#n404
http://git.openstack.org/cgit/openstack/networking-midonet/tree/devstack/magnum/local.conf#n37

They all set the value to 10 instead of the default 60, not sure what the net effect on tests might be.

I think the first one is covered by https://review.openstack.org/652711 , since setting CINDER_PERIODIC_INTERVAL in the local.conf file won't have any effect.

[Brian Rosmaita (brian-rosmaita)]

Patch to remove use of CINDER_PERIODIC_INTERVAL from networking-midonet:
https://review.opendev.org/#/c/655448/

[Brian Rosmaita (brian-rosmaita)]

Patch to remove use of CINDER_PERIODIC_INTERVAL from openstack/devstack-gate:
https://review.opendev.org/655451

[Brian Rosmaita (brian-rosmaita)]

Quick update of what's going on with this bug:

(1) patch to remove CINDER_PERIODIC_INTERVAL from devstack:
https://review.openstack.org/652711
status: waiting to assess the effect of (3) below

(2) patch to remove setting CINDER_PERIODIC_INTERVAL from networking-midonet:
https://review.opendev.org/#/c/655448/
status: approved by midonet team

(3) patch to remove setting CINDER_PERIODIC_INTERVAL from devstack-gate/devstack-vm-gate.sh
https://review.opendev.org/#/c/655451/
status:
devstack-vm-gate.sh is used by the three neutron-grenade* tests on devstack-gate patches.
So we can assess the impact of removing CINDER_PERIODIC_INTERVAL by seeing what happens with those tests.
I put up a set of 6 patches that depend on this change and ran them through the Zuul check twice:
https://review.opendev.org/#/q/topic:assess-655451
The grenade tests passed all 12 times. I left an analysis of the other test failures. Most are unrelated to Cinder. Of the ones that were Cinder-related, none showed the issue that CINDER_PERIODIC_INTERVAL was introduced to address, namely, a failure to build volumes during tests (the Cinder scheduler thought it was out of capacity because deleted volumes weren't being added back to the capacity count quickly enough).

At this point, I think it's safe to merge https://review.openstack.org/652711 and https://review.opendev.org/#/c/655451/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/652711
Committed: https://git.openstack.org/cgit/openstack/devstack/commit/?id=87daf8abe64f9b51fb840a455088b18fd0a791ff
Submitter: Zuul
Branch: master

commit 87daf8abe64f9b51fb840a455088b18fd0a791ff
Author: Brian Rosmaita <email address hidden>
Date: Mon Apr 15 12:00:07 2019 -0400

    End support for changing cinder periodic_interval

    Support for changing the cinder periodic_interval config option
    was added way back in havana as a workaround for bug #1180976
    by change I20e52e66fcc94b224476cdd14c88bd6981b4e617. As the fix
    for that bug does not require modifying this config value, and
    such modification may have unintentional adverse effects, end
    the support.

    Change-Id: I1ef1fe564123216b19582262726cdb1078b7650e
    Partial-bug: #1824837