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)
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)
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
