search

twitter / summingbird

Streaming MapReduce with Scalding and Storm
https://twitter.com/summingbird
Apache License 2.0
2.14k stars 267 forks source link

Extracting Future queuing logic from AsyncBase #685

Closed jnievelt closed 7 years ago

jnievelt commented 8 years ago

Adding FutureQueue, a structure that essentially takes in (S, Future[T]) and gives back (S, Try[T]).

I didn't mean to totally gut AsyncBase, but it's mostly become a shim integrating its own interface (apply, tick), the OperationContainer interface (execute, executeTick), and its FutureQueue.

Making this private for now.

Adding a few more tests while I'm at it. Cleaning up Node.scala style since it does that automatically.

The next step would be to integrate this with AggregatingOutputCollector.

jnievelt commented 7 years ago

These comments should be addressed. On review, there is sort of a dependency on Queue, so maybe the dream of moving this out of summingbird is a bit farfetched?

I'm thinking about moving it up into the online package next to Queue. Mixed feelings about leaving is private vs. not.

jnievelt commented 7 years ago

Any remaining concerns on this one? I've verified that the current diff works fine with my planned changes to AggregatorOutputCollector.

johnynek commented 7 years ago

+1 this is really great work, @jnievelt

I think the FutureQueue is really a general rate-limiter on S => Future[T] now.