You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Zack Shoylev <no...@github.com> on 2015/05/12 02:25:42 UTC
[jclouds] Multipart upload code fixes for swift (#745)
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/745
-- Commit Summary --
* Multipart upload code fixes for swift
-- File Changes --
A apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/binders/BindManifestToJsonPayload.java (65)
M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/binders/BindMetadataToHeaders.java (6)
A apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/binders/BindToHeaders.java (81)
M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStore.java (27)
M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/ObjectApi.java (8)
M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/features/StaticLargeObjectApi.java (40)
M apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/blobstore/integration/SwiftBlobIntegrationLiveTest.java (12)
M apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/features/ObjectApiMockTest.java (2)
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobIntegrationTest.java (1)
M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java (2)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/745.patch
https://github.com/jclouds/jclouds/pull/745.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
I have not yet backported any of these changes since the MPU interface might still be in flux. Waiting to see if GCS throws us any challenges. @danbroudy
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101466168
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import javax.inject.Inject;
> +
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.json.Json;
> +import org.jclouds.rest.MapBinder;
> +
> +/**
> + * Binds the object to the request as a json object.
> + */
> +public class BindManifestToJsonPayload implements MapBinder {
> +
> + protected final Json jsonBinder;
> +
> + @Inject
> + public BindManifestToJsonPayload(Json jsonBinder) {
Make this package private and remove the redundant null check.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30147456
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
> @@ -553,7 +553,7 @@ protected void addTenObjectsUnderPrefix(String containerName, String prefix) thr
>
> protected void awaitConsistency() {
> if (view.getConsistencyModel() == ConsistencyModel.EVENTUAL) {
> - Uninterruptibles.sleepUninterruptibly(10, TimeUnit.SECONDS);
> + Uninterruptibles.sleepUninterruptibly(20, TimeUnit.SECONDS);
I like the idea of using eventuallyTrue for testing, but I thought it was deprecated/removed?
Seems to be the most efficient way to do it, though.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30191171
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -87,4 +114,17 @@ String replaceManifest(@PathParam("objectName") String objectName,
> @Fallback(VoidOnNotFoundOr404.class)
> @QueryParams(keys = "multipart-manifest", values = "delete")
> void delete(@PathParam("objectName") String objectName);
> +
> + /**
> + * Get a static large object's manifest.
> + *
> + * @param objectName
> + * corresponds to {@link SwiftObject#getName()}.
> + *
> + * @return A list of the multipart segments
> + */
> + @Named("staticLargeObject:getManifest")
> + @GET
> + @QueryParams(keys = "multipart-manifest", values = "get")
> + List<Segment> getManifest(@PathParam("objectName") String objectName);
Add the corresponding mock and live tests for the new methods
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30147499
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
> @@ -553,7 +553,7 @@ protected void addTenObjectsUnderPrefix(String containerName, String prefix) thr
>
> protected void awaitConsistency() {
> if (view.getConsistencyModel() == ConsistencyModel.EVENTUAL) {
> - Uninterruptibles.sleepUninterruptibly(10, TimeUnit.SECONDS);
> + Uninterruptibles.sleepUninterruptibly(20, TimeUnit.SECONDS);
Sorry, this is called `RetryablePredicate` now. Although even this is not a guarantee that subsequent operations will succeed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30191997
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
> Can you tag these commits with JCLOUDS-894?
I will try to remember to do this when I rebase :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101445023
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
@zack-shoylev Should this enable the two MPU tests in `SwiftBlobIntegrationLiveTest`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101424465
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
@andrewgaul Let me know if this looks ok now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101421031
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
Makes sense. Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101467386
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
> One spurious failure:
I have seen this twice. A bit hard to reproduce.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101449608
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
> @@ -553,7 +553,7 @@ protected void addTenObjectsUnderPrefix(String containerName, String prefix) thr
>
> protected void awaitConsistency() {
> if (view.getConsistencyModel() == ConsistencyModel.EVENTUAL) {
> - Uninterruptibles.sleepUninterruptibly(10, TimeUnit.SECONDS);
> + Uninterruptibles.sleepUninterruptibly(20, TimeUnit.SECONDS);
It should only slow down tests that test a ConsistencyModel.EVENTUAL
Is this EVENTUAL maybe used in a way I didn't expect?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30187759
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
> @@ -553,7 +553,7 @@ protected void addTenObjectsUnderPrefix(String containerName, String prefix) thr
>
> protected void awaitConsistency() {
> if (view.getConsistencyModel() == ConsistencyModel.EVENTUAL) {
> - Uninterruptibles.sleepUninterruptibly(10, TimeUnit.SECONDS);
> + Uninterruptibles.sleepUninterruptibly(20, TimeUnit.SECONDS);
Can you just override this in `SwiftContainerIntegrationLiveTest` if needed? This sleep is really unfortunate since it slows down all tests unnecessarily, not just the uncommon case of eventual consistency.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30184289
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
merged.
Some conflicts backporting. @andrewgaul - did you backport your code?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101465926
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
:+1:
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101461677
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
> @@ -453,7 +474,7 @@ public MultipartPart uploadMultipartPart(MultipartUpload mpu, int partNumber, Pa
>
> @Override
> public long getMinimumMultipartPartSize() {
> - return 1024 * 1024;
> + return 1024 * 1024 + 1;
Is this needed?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30183316
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
@andrewgaul
I got the tests to work with swift and in the process I think I improved a bit on how the metadata and headers are processed. There is probably more work left, though, and I would like to see an improvement in how the content-* headers are handled.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101080757
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
> @@ -83,14 +83,4 @@ public void testSetBlobAccess() throws Exception {
> public void testCopyBlobReplaceMetadata() throws Exception {
> throw new SkipException("Swift only supports appending to user metadata, not replacing it");
> }
> -
> - @Override
> - public void testMultipartUploadSinglePart() throws Exception {
> - throw new SkipException("openstack-swift does not support setting blob metadata during multipart upload");
> - }
> -
> - @Override
> - public void testMultipartUploadMultipleParts() throws Exception {
> - throw new SkipException("openstack-swift does not support setting blob metadata during multipart upload");
> - }
Disregard earlier comment -- I missed this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30183350
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
Successfully ran integration tests locally. One spurious failure:
```
CloudFilesUSContainerIntegrationLiveTest>BaseContainerIntegrationTest.testDirectory:244 [[type=BLOB, id=null, name=directory/0, location=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore6633675103555677139/directory/0, userMetadata={}], [type=BLOB, id=null, name=directory/1, location=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore6633675103555677139/directory/1, userMetadata={}], [type=BLOB, id=null, name=directory/2, location=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore6633675103555677139/directory/2, userMetadata={}], [type=BLOB, id=null, name=directory/3, location=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore6633675103555677139/directory/3, userMetadata={}], [type=BLOB, id=null, name=directory/4,
location
=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore6633675103555677139/directory/4, userMetadata={}], [type=BLOB, id=null, name=directory/5, location=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore6633675103555677139/directory/5, userMetadata={}], [type=BLOB, id=null, name=directory/6, location=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore6633675103555677139/directory/6, userMetadata={}], [type=BLOB, id=null, name=directory/8, location=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore6633675103555677139/directory/8, userMetadata={}], [type=BLOB, id=null, name=directory/9, location=null, uri=https://storage101.dfw1.clouddrive.com/v1/MossoCloudFS_e2d63902-0d79-417c-90d8-5603fac57bbb/gaul-blobstore663367510355
5677139/
directory/9, userMetadata={}]]
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101446052
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
TODO: backport
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101465971
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Zack Shoylev <no...@github.com>.
Closed #745.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#event-303374279
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
> @@ -553,7 +553,7 @@ protected void addTenObjectsUnderPrefix(String containerName, String prefix) thr
>
> protected void awaitConsistency() {
> if (view.getConsistencyModel() == ConsistencyModel.EVENTUAL) {
> - Uninterruptibles.sleepUninterruptibly(10, TimeUnit.SECONDS);
> + Uninterruptibles.sleepUninterruptibly(20, TimeUnit.SECONDS);
tl;dr we should not increase the timeout since this increases test run time for most providers. If needed, add an override method to the Swift provider.
lots of free time edition: we should not sleep at all. Instead we should call `Utils.eventuallyTrue` which breaks as soon as the condition is met. Most operations do not exhibit eventual consistency and I observed the most inconsistent provider exhibit it only 3% of the time:
https://github.com/andrewgaul/are-we-consistent-yet
I have seen eventual consistency take over 1,000 seconds to reconcile so bumping from 10 to 20 is not much of a guarantee.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745/files#r30190424
Re: [jclouds] Multipart upload code fixes for swift (#745)
Posted by Andrew Gaul <no...@github.com>.
Can you tag these commits with JCLOUDS-894?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/745#issuecomment-101431735