From 16f1b7064df27c251585b897f5a88f7d73ebea74 Mon Sep 17 00:00:00 2001 From: Stuart McLaren Date: Mon, 25 Jan 2016 15:11:18 +0000 Subject: Change Swift zero-size chunk behaviour There is a difference in how Swift and radosgw behave when a zero length write is aborted. Swift returns 499 and doesn't create an object, but radosgw appears to creates a zero size object. Because of this, new behaviour which was introduced to prevent zero-size chunks being written worked for Swift, but appears not to work for the radosgw case. This patch changes how we detect a zero size chunk so that no request is made to the backend. We no longer rely on the Swift specific behaviour. Change-Id: I1a48e2eceaa6b07b8e5bbfd42fef6fd3d91f5834 Closes-bug: 1537721 --- glance_store/_drivers/swift/store.py | 22 +++++++++++------ glance_store/exceptions.py | 4 --- glance_store/tests/unit/test_swift_store.py | 38 +++++++++++++++++++++++------ 3 files changed, 45 insertions(+), 19 deletions(-) diff --git a/glance_store/_drivers/swift/store.py b/glance_store/_drivers/swift/store.py index 2f305df..94bf20d 100644 --- a/glance_store/_drivers/swift/store.py +++ b/glance_store/_drivers/swift/store.py @@ -546,14 +546,14 @@ class BaseStore(driver.Store): chunk_name = "%s-%05d" % (location.obj, chunk_id) reader = ChunkReader(image_file, checksum, chunk_size, verifier) + if reader.is_zero_size is True: + LOG.debug('Not writing zero-length chunk.') + break try: chunk_etag = connection.put_object( location.container, chunk_name, reader, content_length=content_length) written_chunks.append(chunk_name) - except exceptions.ZeroSizeChunk: - LOG.debug('Not writing zero-length chunk') - break except Exception: # Delete orphaned segments from swift backend with excutils.save_and_reraise_exception(): @@ -952,15 +952,23 @@ class ChunkReader(object): self.total = total self.verifier = verifier self.bytes_read = 0 + self.is_zero_size = False + self.byteone = fd.read(1) + if len(self.byteone) == 0: + self.is_zero_size = True + + def do_read(self, i): + if self.bytes_read == 0 and i > 0 and self.byteone is not None: + return self.byteone + self.fd.read(i - 1) + else: + return self.fd.read(i) def read(self, i): left = self.total - self.bytes_read if i > left: i = left - result = self.fd.read(i) - if len(result) == 0 and self.bytes_read == 0: - # fd was empty - raise exceptions.ZeroSizeChunk() + + result = self.do_read(i) self.bytes_read += len(result) self.checksum.update(result) if self.verifier: diff --git a/glance_store/exceptions.py b/glance_store/exceptions.py index 163e3e9..fe59630 100644 --- a/glance_store/exceptions.py +++ b/glance_store/exceptions.py @@ -83,10 +83,6 @@ class NotFound(GlanceStoreException): message = _("Image %(image)s not found") -class ZeroSizeChunk(GlanceStoreException): - message = _("Zero size chunk") - - class UnknownScheme(GlanceStoreException): message = _("Unknown scheme '%(scheme)s' found in URI") diff --git a/glance_store/tests/unit/test_swift_store.py b/glance_store/tests/unit/test_swift_store.py index 7268952..87e252a 100644 --- a/glance_store/tests/unit/test_swift_store.py +++ b/glance_store/tests/unit/test_swift_store.py @@ -778,11 +778,8 @@ class SwiftTests(object): self.assertEqual(expected_location, loc) self.assertEqual(expected_swift_size, size) self.assertEqual(expected_checksum, checksum) - # Expecting 7 calls to put_object -- 5 chunks, a zero chunk which is - # then deleted, and the manifest. Note the difference with above - # where the image_size is specified in advance (there's no zero chunk - # in that case). - self.assertEqual(SWIFT_PUT_OBJECT_CALLS, 7) + # Expecting 6 calls to put_object -- 5 chunks, and the manifest. + self.assertEqual(SWIFT_PUT_OBJECT_CALLS, 6) loc = location.get_location_from_uri(expected_location, conf=self.conf) (new_image_swift, new_image_size) = self.store.get(loc) @@ -1554,12 +1551,37 @@ class TestChunkReader(base.StoreBaseTest): bytes_read = 0 while True: cr = swift.ChunkReader(infile, checksum, CHUNKSIZE) - try: - chunk = cr.read(CHUNKSIZE) - except exceptions.ZeroSizeChunk: + chunk = cr.read(CHUNKSIZE) + if len(chunk) == 0: + self.assertEqual(True, cr.is_zero_size) break bytes_read += len(chunk) self.assertEqual(units.Ki, bytes_read) + self.assertEqual('fb10c6486390bec8414be90a93dfff3b', + cr.checksum.hexdigest()) + data_file.close() + infile.close() + + def test_read_zero_size_data(self): + """ + Replicate what goes on in the Swift driver with the + repeated creation of the ChunkReader object + """ + CHUNKSIZE = 100 + checksum = hashlib.md5() + data_file = tempfile.NamedTemporaryFile() + infile = open(data_file.name, 'rb') + bytes_read = 0 + while True: + cr = swift.ChunkReader(infile, checksum, CHUNKSIZE) + chunk = cr.read(CHUNKSIZE) + if len(chunk) == 0: + break + bytes_read += len(chunk) + self.assertEqual(True, cr.is_zero_size) + self.assertEqual(0, bytes_read) + self.assertEqual('d41d8cd98f00b204e9800998ecf8427e', + cr.checksum.hexdigest()) data_file.close() infile.close() -- cgit v1.2.1