jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ignasi Barrera <notificati...@github.com>
Subject [jclouds] JCLOUDS-532: Properly close HTTP streams (#700)
Date Mon, 09 Mar 2015 11:51:15 GMT
https://issues.apache.org/jira/browse/JCLOUDS-532

This commit moves the responsibility of closing the HTTP streams to the base class for the
command execution services (which drivers inherit from), instead of being in a concrete implementation
of a retry handler.

If the request has to be retried, we just close the stream and execute the request again.
If the request cannot be retried and is not in the 2xx range, then we buffer and close the
stream and pass it to the error handler. We could delegate the responsibility of closing the
stream to the error handler without buffering it first (actually most, if not all, error handlers
take care of closing the stream), but in the current implementation of the default driver,
the error stream was buffered by default, so it seems safe to apply that behavior to the driver's
base class, and remove that responsibility from each provider's error handler. Error streams
shouldn't be big enough to present memory issues.

@andrewgaul This is a sensible change, specially for BlobStore providers, which make an extensive
use of the response payloads. Could you run a smoke/live test on the common providers with
the changes in this branch?

/cc @zack-shoylev 
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * JCLOUDS-532: Properly close HTTP streams

-- File Changes --

    M core/src/main/java/org/jclouds/http/handlers/BackoffLimitedRetryHandler.java (2)
    M core/src/main/java/org/jclouds/http/internal/BaseHttpCommandExecutorService.java (13)
    M core/src/main/java/org/jclouds/http/internal/JavaUrlHttpCommandExecutorService.java
(16)
    M core/src/test/java/org/jclouds/http/handlers/BackoffLimitedRetryHandlerTest.java (31)
    A core/src/test/java/org/jclouds/http/internal/BaseHttpCommandExecutorServiceTest.java
(185)

-- Patch Links --

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

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/700

Mime
View raw message