From: Tom Barron Date: Thu, 7 May 2015 19:46:06 +0000 (-0400) Subject: Handle ineffective backup compression X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fc5affbe757099011481678e74e0f19d5200424d;p=openstack-build%2Fcinder-build.git Handle ineffective backup compression Don't compress chunks when compressed length is not a gain over original, uncompressed length. With incremental backups, where chunks can be built from short "extents", the likelihood of this situation has significantly increased. Closes-Bug: 1462268 Change-Id: Id960cb22ae74f19154f49a0c19ee07a00145067f --- diff --git a/cinder/backup/chunkeddriver.py b/cinder/backup/chunkeddriver.py index d9cfce54e..23738c578 100644 --- a/cinder/backup/chunkeddriver.py +++ b/cinder/backup/chunkeddriver.py @@ -297,30 +297,14 @@ class ChunkedBackupDriver(driver.BackupDriver): obj[object_name] = {} obj[object_name]['offset'] = data_offset obj[object_name]['length'] = len(data) - LOG.debug('reading chunk of data from volume') - if self.compressor is not None: - algorithm = CONF.backup_compression_algorithm.lower() - obj[object_name]['compression'] = algorithm - data_size_bytes = len(data) - data = self.compressor.compress(data) - comp_size_bytes = len(data) - LOG.debug('compressed %(data_size_bytes)d bytes of data ' - 'to %(comp_size_bytes)d bytes using ' - '%(algorithm)s', - { - 'data_size_bytes': data_size_bytes, - 'comp_size_bytes': comp_size_bytes, - 'algorithm': algorithm, - }) - else: - LOG.debug('not compressing data') - obj[object_name]['compression'] = 'none' - + LOG.debug('Backing up chunk of data from volume.') + algorithm, output_data = self._prepare_output_data(data) + obj[object_name]['compression'] = algorithm LOG.debug('About to put_object') with self.get_object_writer( container, object_name, extra_metadata=extra_metadata ) as writer: - writer.write(data) + writer.write(output_data) md5 = hashlib.md5(data).hexdigest() obj[object_name]['md5'] = md5 LOG.debug('backup MD5 for %(object_name)s: %(md5)s', @@ -333,6 +317,30 @@ class ChunkedBackupDriver(driver.BackupDriver): LOG.debug('Calling eventlet.sleep(0)') eventlet.sleep(0) + def _prepare_output_data(self, data): + if self.compressor is None: + return 'none', data + data_size_bytes = len(data) + compressed_data = self.compressor.compress(data) + comp_size_bytes = len(compressed_data) + algorithm = CONF.backup_compression_algorithm.lower() + if comp_size_bytes >= data_size_bytes: + LOG.debug('Compression of this chunk was ineffective: ' + 'original length: %(data_size_bytes)d, ' + 'compressed length: %(compressed_size_bytes)d. ' + 'Using original data for this chunk.', + {'data_size_bytes': data_size_bytes, + 'comp_size_bytes': comp_size_bytes, + }) + return 'none', data + LOG.debug('Compressed %(data_size_bytes)d bytes of data ' + 'to %(comp_size_bytes)d bytes using %(algorithm)s.', + {'data_size_bytes': data_size_bytes, + 'comp_size_bytes': comp_size_bytes, + 'algorithm': algorithm, + }) + return algorithm, compressed_data + def _finalize_backup(self, backup, container, object_meta, object_sha256): """Write the backup's metadata to the backup repository.""" object_list = object_meta['list'] diff --git a/cinder/tests/unit/backup/drivers/test_backup_nfs.py b/cinder/tests/unit/backup/drivers/test_backup_nfs.py index 812a6d9d3..b77317728 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_nfs.py +++ b/cinder/tests/unit/backup/drivers/test_backup_nfs.py @@ -715,3 +715,36 @@ class BackupNFSSwiftBasedTestCase(test.TestCase): compressor = service._get_compressor('bz2') self.assertEqual(compressor, bz2) self.assertRaises(ValueError, service._get_compressor, 'fake') + + def test_prepare_output_data_effective_compression(self): + service = nfs.NFSBackupDriver(self.ctxt) + # Set up buffer of 128 zeroed bytes + fake_data = buffer(bytearray(128)) + + result = service._prepare_output_data(fake_data) + + self.assertEqual('zlib', result[0]) + self.assertTrue(len(result) < len(fake_data)) + + def test_prepare_output_data_no_compresssion(self): + self.flags(backup_compression_algorithm='none') + service = nfs.NFSBackupDriver(self.ctxt) + # Set up buffer of 128 zeroed bytes + fake_data = buffer(bytearray(128)) + + result = service._prepare_output_data(fake_data) + + self.assertEqual('none', result[0]) + self.assertEqual(fake_data, result[1]) + + def test_prepare_output_data_ineffective_compression(self): + service = nfs.NFSBackupDriver(self.ctxt) + # Set up buffer of 128 zeroed bytes + fake_data = buffer(bytearray(128)) + # Pre-compress so that compression in the driver will be ineffective. + already_compressed_data = service.compressor.compress(fake_data) + + result = service._prepare_output_data(already_compressed_data) + + self.assertEqual('none', result[0]) + self.assertEqual(already_compressed_data, result[1]) diff --git a/cinder/tests/unit/test_backup_swift.py b/cinder/tests/unit/test_backup_swift.py index 70a72afa9..3f0afe3b1 100644 --- a/cinder/tests/unit/test_backup_swift.py +++ b/cinder/tests/unit/test_backup_swift.py @@ -561,3 +561,36 @@ class BackupSwiftTestCase(test.TestCase): compressor = service._get_compressor('bz2') self.assertEqual(compressor, bz2) self.assertRaises(ValueError, service._get_compressor, 'fake') + + def test_prepare_output_data_effective_compression(self): + service = swift_dr.SwiftBackupDriver(self.ctxt) + # Set up buffer of 128 zeroed bytes + fake_data = buffer(bytearray(128)) + + result = service._prepare_output_data(fake_data) + + self.assertEqual('zlib', result[0]) + self.assertTrue(len(result) < len(fake_data)) + + def test_prepare_output_data_no_compresssion(self): + self.flags(backup_compression_algorithm='none') + service = swift_dr.SwiftBackupDriver(self.ctxt) + # Set up buffer of 128 zeroed bytes + fake_data = buffer(bytearray(128)) + + result = service._prepare_output_data(fake_data) + + self.assertEqual('none', result[0]) + self.assertEqual(fake_data, result[1]) + + def test_prepare_output_data_ineffective_compression(self): + service = swift_dr.SwiftBackupDriver(self.ctxt) + # Set up buffer of 128 zeroed bytes + fake_data = buffer(bytearray(128)) + # Pre-compress so that compression in the driver will be ineffective. + already_compressed_data = service.compressor.compress(fake_data) + + result = service._prepare_output_data(already_compressed_data) + + self.assertEqual('none', result[0]) + self.assertEqual(already_compressed_data, result[1])