Fail fast implementation for joinList

[JoseAlavez]

Hi, we use this library on one of the projects I work on and I noticed an undesirable behavior that gets improved with this PR.

Context:

We use this library in combination with Netty, to consume REST APIs that provide us with single fetch contracts (as for example GET /domain/resource/{id}). In other words, when we require to fetch a batch of resources by id, we create individual asynchronous HTTP calls to the respective APIs and then join the completable futures using .collect(CompletableFutures.joinList()).

When our app is under heavy load and our fetch batch logic leads to individual HTTP calls that grows dramatically, we are seeing execution times that are higher than the timeouts specified for each completable future. This is caused due to the capacity of our executor to perform each HTTP call (and queuing) but also because the current implementation of joinList and allAsList utilizes CompletableFuture#allOf, which will wait until all the states are completed to either continue normally or exceptionally.

Proposal

I took the chance of implementing a extendable fail fast behavior in the joinList and allAsList methods. An interface that can have multiple implementations can be passed as parameter to define how and when the combined completable future needs to fail fast (complete exceptionally) once one or more stages fail. Furthermore, the execution of the incomplete stages can be cancelled when failing fast (understanding that it will cancel further stage chaining and not the current execution of the stage).

Side Notes:

• I tried to honor as much the current coding style and practices (as for example, favoring conventional for statements or snake case on test method names for CompletableFuturesTest.java). Please advise if this is yet necessary or if the code can be simplified.
• Checking stackoverflow.com seems there is a need in the community for this behavior to be implemented (ref1, ref2). I thought this library is a nice candidate to contain a solution for it.

Let me know if this PR brings value to the code base (as we look forward to use it in our project), or if anything here needs to be adapted or if the request would be discarded.

[codecov]

Codecov Report

Merging #68 into master will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##              master       #68   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity        84        87    +3     
===========================================
  Files              7         7           
  Lines            222       228    +6     
  Branches          16        17    +1     
===========================================
+ Hits             222       228    +6     

Impacted Files	Coverage Δ	Complexity Δ
...n/java/com/spotify/futures/CompletableFutures.java	100.00% <100.00%> (ø)	52.00 <6.00> (+3.00)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a318f8...be04285. Read the comment docs.

[spkrka]

Since @mbruggmann and @dflemstr have been the primary developers of this library, I added them as reviewers.

[JoseAlavez]

@mbruggmann @dflemstr @spkrka I have dropped all the excess and left the essential core of what I was after.

I'm still looking forward to cancel the remaining incomplete tasks when failing fast, but I will prefer to do this in another PR and after discussing with you guys whether new method or parameters would be ideal to extend your API.

Sorry for the long discussions and my sloppy approach with the initial PR review. Thanks for the feedback!

[dflemstr]

Looks reasonable, but I'll wait for the other two to review.

[mbruggmann]

Nice, I like how this boiled down to something that benefits everyone even without an API change!

[spkrka]

Really nice, short and sweet!

[mbruggmann]

Thanks a lot for your contribution @JoseAlavez 🎉