openstack
/
cinder
Code Issues Proposed changes

Revert "Cleanup code duplication in cinder.cmd.backup module" 

This reverts commit f6c60d1d9c.

The original patch referred as duplicated code the part where the
service uses a single process when there is no additional processes
requested (backup_workers == 1), which is the default.

That code made the cinder-backup service always run as a child process
even when there was only going to be 1 process.

This adds 30MB of extra RAM consumption by the service, which amounts to
a 30% unnecessary memory increase when not doing operations after
applying changes from Ic9030d01468b3189350f83b04a8d1d346c489d3c.

Change-Id: I43a20c8687f12bc52b014611cc6977c4c3ca212c
This commit is contained in:
Gorka Eguileor 2022-06-14 20:03:58 +02:00
parent 3dfc519301
commit 5c7fd3a70b
2 changed files with 40 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
cinder/cmd/backup.py
View File

@ -82,7 +82,7 @@ def _launch_backup_process(launcher, num_process, _semaphore):
server = service.Service.create(binary='cinder-backup',
coordination=True,
service_name='backup',
process_number=num_process,
process_number=num_process + 1,
semaphore=_semaphore)
except Exception:
LOG.exception('Backup service %s failed to start.', CONF.host)
@ -113,10 +113,20 @@ def main():
semaphore = utils.semaphore_factory(CONF.backup_max_operations,
CONF.backup_workers)
LOG.info('Backup running with %s processes.', CONF.backup_workers)
launcher = service.get_launcher()
if CONF.backup_workers > 1:
LOG.info('Backup running with %s processes.', CONF.backup_workers)
launcher = service.get_launcher()
for i in range(1, CONF.backup_workers + 1):
_launch_backup_process(launcher, i, semaphore)
for i in range(CONF.backup_workers):
_launch_backup_process(launcher, i, semaphore)
launcher.wait()
launcher.wait()
else:
LOG.info('Backup running in single process mode.')
server = service.Service.create(binary='cinder-backup',
coordination=True,
service_name='backup',
process_number=1,
semaphore=semaphore)
service.serve(server)
service.wait()

24
cinder/tests/unit/test_cmd.py
View File

@ -97,6 +97,30 @@ class TestCinderBackupCmd(test.TestCase):
super(TestCinderBackupCmd, self).setUp()
sys.argv = ['cinder-backup']
@mock.patch('cinder.utils.semaphore_factory')
@mock.patch('cinder.cmd.backup._launch_backup_process')
@mock.patch('cinder.service.wait')
@mock.patch('cinder.service.serve')
@mock.patch('cinder.service.Service.create')
@mock.patch('cinder.utils.monkey_patch')
@mock.patch('oslo_log.log.setup')
def test_main(self, log_setup, monkey_patch, service_create, service_serve,
service_wait, launch_mock, mock_semaphore):
server = service_create.return_value
cinder_backup.main()
self.assertEqual('cinder', CONF.project)
self.assertEqual(CONF.version, version.version_string())
log_setup.assert_called_once_with(CONF, "cinder")
monkey_patch.assert_called_once_with()
service_create.assert_called_once_with(
binary='cinder-backup', coordination=True, service_name='backup',
process_number=1, semaphore=mock_semaphore.return_value)
service_serve.assert_called_once_with(server)
service_wait.assert_called_once_with()
launch_mock.assert_not_called()
@mock.patch('cinder.utils.Semaphore')
@mock.patch('cinder.service.get_launcher')
@mock.patch('cinder.service.Service.create')