You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Timur Alperovich <no...@github.com> on 2020/03/04 20:17:36 UTC

[jclouds/jclouds] Hash the content for fs MPU ETag if no xattr. (#1285)

If there is no extended attribute support in the file system, the blobs
will not have their associated ETags available. In that case, the file
system blob store should rehash the content while producing the combined
blob and return the expected S3-style ETag.
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/1285

-- Commit Summary --

  * Hash the content for fs MPU ETag if no xattr.

-- File Changes --

    M blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java (62)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/1285.patch
https://github.com/jclouds/jclouds/pull/1285.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1285

Re: [jclouds/jclouds] Hash the content for fs MPU ETag if no xattr. (#1285)

Posted by Timur Alperovich <no...@github.com>.
Updated to return an empty-string MD5 if the ETag attribute of a blob is null.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1285#issuecomment-608569542

Re: [jclouds/jclouds] Hash the content for fs MPU ETag if no xattr. (#1285)

Posted by Timur Alperovich <no...@github.com>.
timuralp commented on this pull request.



>  
       for (MultipartPart part : parts) {
          Blob blobPart = getBlob(mpu.containerName(), MULTIPART_PREFIX + mpu.id() + "-" + mpu.blobName() + "-" + part.partNumber());
          contentLength += blobPart.getMetadata().getContentMetadata().getContentLength();
          blobs.add(blobPart);
-         md5Hasher.putBytes(BaseEncoding.base16().lowerCase().decode(blobPart.getMetadata().getETag()));
+         if (blobPart.getMetadata().getETag() != null) {
+            md5Hasher.putBytes(BaseEncoding.base16().lowerCase().decode(blobPart.getMetadata().getETag()));

I did consider that and thought that previously the blob store returned the hash of the entire content on PUT. Looking back, it looks like it actually returned the hash of an empty string (as the blob part ETag didn't exist). I'll change this patch to the same behavior, keeping the `-{number of parts}` suffix for consistency.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1285#discussion_r388117257

Re: [jclouds/jclouds] Hash the content for fs MPU ETag if no xattr. (#1285)

Posted by Andrew Gaul <no...@github.com>.
gaul commented on this pull request.



>  
       for (MultipartPart part : parts) {
          Blob blobPart = getBlob(mpu.containerName(), MULTIPART_PREFIX + mpu.id() + "-" + mpu.blobName() + "-" + part.partNumber());
          contentLength += blobPart.getMetadata().getContentMetadata().getContentLength();
          blobs.add(blobPart);
-         md5Hasher.putBytes(BaseEncoding.base16().lowerCase().decode(blobPart.getMetadata().getETag()));
+         if (blobPart.getMetadata().getETag() != null) {
+            md5Hasher.putBytes(BaseEncoding.base16().lowerCase().decode(blobPart.getMetadata().getETag()));

You might also want to look at `listMultipartUpload`.  Recalculating ETags can be expensive if a client calls this in some kind of loop!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1285#discussion_r388202683

Re: [jclouds/jclouds] Hash the content for fs MPU ETag if no xattr. (#1285)

Posted by Andrew Gaul <no...@github.com>.
Merged #1285 into master.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1285#event-3199334197

Re: [jclouds/jclouds] Hash the content for fs MPU ETag if no xattr. (#1285)

Posted by Andrew Gaul <no...@github.com>.
Sorry this should have been opened against apache/jclouds repository, not jclouds/jclouds.  I pushed this to the correct master as 8d0e9dc962dff614d56ca7428bbdca5462679c96 and 2.2.x as e66bffa52168fa1c005b84477d9a4754c7a57074.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1285#issuecomment-609004130

Re: [jclouds/jclouds] Hash the content for fs MPU ETag if no xattr. (#1285)

Posted by Andrew Gaul <no...@github.com>.
gaul commented on this pull request.



>  
       for (MultipartPart part : parts) {
          Blob blobPart = getBlob(mpu.containerName(), MULTIPART_PREFIX + mpu.id() + "-" + mpu.blobName() + "-" + part.partNumber());
          contentLength += blobPart.getMetadata().getContentMetadata().getContentLength();
          blobs.add(blobPart);
-         md5Hasher.putBytes(BaseEncoding.base16().lowerCase().decode(blobPart.getMetadata().getETag()));
+         if (blobPart.getMetadata().getETag() != null) {
+            md5Hasher.putBytes(BaseEncoding.base16().lowerCase().decode(blobPart.getMetadata().getETag()));

This seems reasonable, but did you consider returning an empty ETag instead?  I appreciate the commitment to S3 fidelity buy `FilesystemStorageStrategyImpl.getBlob` already returns null ETags instead of recalculating them.  Systems without extended attributes are uncommon so is the additional complexity worth it?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1285#pullrequestreview-369234774

Re: [jclouds/jclouds] Hash the content for fs MPU ETag if no xattr. (#1285)

Posted by Timur Alperovich <no...@github.com>.
@timuralp pushed 1 commit.

7970cf43fad93571b9ff4bd8113a6d1d989a906a  Hash the content for fs MPU ETag if no xattr.


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds/pull/1285/files/f7dccfd9f2353a666456cd9a8fb667d47de9affd..7970cf43fad93571b9ff4bd8113a6d1d989a906a