Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
Closed #1010. -- 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/1010#event-789701771
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
merged -- 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/1010#issuecomment-247129173
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
andrewgaul approved this pull request. -- 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/1010#pullrequestreview-29746
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
>I will try to do a separate PR for it (out of scope for this one). Great :) Thanks! -- 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/1010#issuecomment-247084474
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
@nacx 1176: I will try to do a separate PR for it (out of scope for this one). -- 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/1010#issuecomment-247083691
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
Does this also fix JCLOUDS-1176? (Just asking to understand the scope of this PR :)) -- 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/1010#issuecomment-247080469
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
@zack-shoylev pushed 1 commit. c18747d review fixes -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds/pull/1010/files/304c72c3e24e376cebbb98bd3b8f8ad6ecd72dac..c18747de0aeb80b3a6ab9eae5c6ceddf6c8e3a1a
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
> @@ -753,8 +770,24 @@ public Void call() { > return null; > } > throw new RuntimeException("After " + retryCountLimit + " retries: > " + lastException); > + } > + > + // JDK-4715154 > + private void closeDirectBuffer(MappedByteBuffer mbb) { logger is not static -- 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/1010/files/304c72c3e24e376cebbb98bd3b8f8ad6ecd72dac#r78770058
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
> @@ -753,8 +770,24 @@ public Void call() { > return null; > } > throw new RuntimeException("After " + retryCountLimit + " retries: > " + lastException); > + } > + > + // JDK-4715154 > + private void closeDirectBuffer(MappedByteBuffer mbb) { > + if ( mbb == null || !mbb.isDirect() ) > +return; > + > + try { > +Method cleaner = mbb.getClass().getMethod("cleaner"); Pretty sure only Windows. Could add an OS check. -- 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/1010/files/304c72c3e24e376cebbb98bd3b8f8ad6ecd72dac#r78688977
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
> } > +} catch (Exception e) { > + logger.debug(e.toString()); > + // close pipe so client is notified of an exception > + Closeables2.closeQuietly(input); > + Closeables2.closeQuietly(output); Would these work better in a `finally` block? -- 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/1010/files/304c72c3e24e376cebbb98bd3b8f8ad6ecd72dac#r78608690
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
> @@ -57,7 +57,7 @@ > public class RegionScopedSwiftBlobStoreParallelLiveTest extends > BaseBlobStoreIntegrationTest { > > private final File BIG_FILE = new File("random.dat"); > - private final long SIZE = 10; //10 * 1000 * 1000; > + private final long SIZE = 1000; //10 * 1000 * 1000; Instead of the comment, could you just say: ```java private static final long SIZE = 10 * 1000 * 1000; ``` -- 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/1010/files/304c72c3e24e376cebbb98bd3b8f8ad6ecd72dac#r78608484
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
> @@ -753,8 +770,24 @@ public Void call() { > return null; > } > throw new RuntimeException("After " + retryCountLimit + " retries: > " + lastException); > + } > + > + // JDK-4715154 > + private void closeDirectBuffer(MappedByteBuffer mbb) { > + if ( mbb == null || !mbb.isDirect() ) > +return; > + > + try { > +Method cleaner = mbb.getClass().getMethod("cleaner"); Do we need to run this on all systems or just Windows? -- 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/1010/files/304c72c3e24e376cebbb98bd3b8f8ad6ecd72dac#r78608199
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
> @@ -753,8 +770,24 @@ public Void call() { > return null; > } > throw new RuntimeException("After " + retryCountLimit + " retries: > " + lastException); > + } > + > + // JDK-4715154 > + private void closeDirectBuffer(MappedByteBuffer mbb) { `static`? -- 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/1010/files/304c72c3e24e376cebbb98bd3b8f8ad6ecd72dac#r78608102
Re: [jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
> @@ -706,12 +709,23 @@ public void downloadBlob(String container, String name, > File destination, Execut > > Futures.getUnchecked(Futures.allAsList(results)); > > + raf.getChannel().force(true); > + raf.getChannel().close(); > + raf.close(); > + > + if ( destination.exists() ) > +destination.delete(); > + if ( !tempFile.renameTo(destination) ) { Please remove extra whitespace. -- 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/1010/files/304c72c3e24e376cebbb98bd3b8f8ad6ecd72dac#r78608020
[jclouds/jclouds] More fixes to parallel download resource cleanup (#1010)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/1010 -- Commit Summary -- * More fixes to parallel download resource cleanup -- File Changes -- M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStore.java (59) M apis/openstack-swift/src/test/java/org/jclouds/openstack/swift/v1/blobstore/RegionScopedSwiftBlobStoreParallelLiveTest.java (3) -- Patch Links -- https://github.com/jclouds/jclouds/pull/1010.patch https://github.com/jclouds/jclouds/pull/1010.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/1010