-
Notifications
You must be signed in to change notification settings - Fork 266
Put all Collector actions in one execute #365
Changes from 11 commits
a002ee1
e4cdbd7
403e1a0
e3e86e7
f4ad6f5
5b571ee
4938ff1
446a7e1
63b3bbf
ca4beba
d26e3a0
e53e2e9
37988e7
b5b2045
6531f74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
/* | ||
Copyright 2013 Twitter, Inc. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package com.twitter.summingbird.online | ||
|
||
import com.twitter.util.{ Await, Duration, Future, Try } | ||
|
||
import java.util.Queue | ||
import java.util.concurrent.{ArrayBlockingQueue, BlockingQueue, LinkedBlockingQueue, TimeUnit} | ||
import java.util.concurrent.ConcurrentLinkedQueue | ||
import java.util.concurrent.atomic.AtomicInteger | ||
/** | ||
* | ||
* @author Oscar Boykin | ||
*/ | ||
|
||
object Channel { | ||
/** | ||
* By default, don't block on put | ||
*/ | ||
def apply[T]() = linkedNonBlocking[T] | ||
|
||
def arrayBlocking[T](size: Int): Channel[T] = | ||
fromBlocking(new ArrayBlockingQueue(size)) | ||
|
||
def linkedBlocking[T]: Channel[T] = | ||
fromBlocking(new LinkedBlockingQueue()) | ||
|
||
def linkedNonBlocking[T]: Channel[T] = | ||
fromQueue(new ConcurrentLinkedQueue()) | ||
|
||
def fromBlocking[T](bq: BlockingQueue[T]): Channel[T] = { | ||
new Channel[T] { | ||
override def add(t: T) = bq.put(t) | ||
override def pollOrNull = bq.poll() | ||
} | ||
} | ||
|
||
// Uses Queue.add to put. This will fail for full blocking queues | ||
def fromQueue[T](q: Queue[T]): Channel[T] = { | ||
new Channel[T] { | ||
override def add(t: T) = q.add(t) | ||
override def pollOrNull = q.poll() | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Use this class with a thread-safe queue to receive | ||
* results from futures in one thread. | ||
* Storm needs us to touch it's code in one event path (via | ||
* the execute method in bolts) | ||
*/ | ||
abstract class Channel[T] { | ||
|
||
// always increment after this | ||
protected def add(t: T): Unit | ||
// always decrement after this | ||
protected def pollOrNull: T | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If its an always, why not bake the behavior into these rather than at touch points? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we supply a more scala idiomatic one, return Option[T] and internally do the decrement if not null? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am trying to make it easy to get that right, but if we require every subclass to call decrement, some will get it wrong. By contrast, if we do it below, we can audit it and test it once. This method is just for the adapter to existing java classes. Option I'll do. That was we being OCD about allocations.... bad move. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So with the the atomic thing, i was thinking it might be more have the downstream api but then final functions which call them and do the decrement/increment. Such that there is only 2 places we need to ensure correctness. Not a big thing, its only a tiny class anyway. |
||
|
||
private val count = new AtomicInteger(0) | ||
|
||
def put(item: T): Int = { | ||
add(item) | ||
count.incrementAndGet | ||
} | ||
|
||
/** Returns the size immediately after the put */ | ||
def putAll(items: TraversableOnce[T]): Int = { | ||
val added = items.foldLeft(0) { (cnt, item) => | ||
add(item) | ||
cnt + 1 | ||
} | ||
count.addAndGet(added) | ||
} | ||
|
||
/** | ||
* check if something is ready now | ||
*/ | ||
def poll: Option[T] = pollOrNull match { | ||
case null => None | ||
case item => | ||
count.decrementAndGet | ||
Some(item) | ||
} | ||
|
||
/** | ||
* Obviously, this might not be the same by the time you | ||
* call spill | ||
*/ | ||
def size: Int = count.get | ||
|
||
// Do something on all the elements ready: | ||
@annotation.tailrec | ||
final def foreach(fn: T => Unit): Unit = | ||
pollOrNull match { | ||
case null => () | ||
case itt => | ||
count.decrementAndGet | ||
fn(itt) | ||
foreach(fn) | ||
} | ||
|
||
// fold on all the elements ready: | ||
@annotation.tailrec | ||
final def foldLeft[V](init: V)(fn: (V, T) => V): V = { | ||
pollOrNull match { | ||
case null => init | ||
case itt => | ||
count.decrementAndGet | ||
foldLeft(fn(init, itt))(fn) | ||
} | ||
} | ||
|
||
/** | ||
* Take enough elements to get the queue under the maxLength | ||
*/ | ||
def trimTo(maxLength: Int): Seq[T] = { | ||
require(maxLength >= 0, "maxLength must be >= 0.") | ||
|
||
@annotation.tailrec | ||
def loop(size: Int, acc: List[T] = Nil): List[T] = { | ||
if(size > maxLength) { | ||
pollOrNull match { | ||
case null => acc.reverse // someone else cleared us out | ||
case item => | ||
loop(count.decrementAndGet, item::acc) | ||
} | ||
} | ||
else acc.reverse | ||
} | ||
loop(count.get) | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
/* | ||
Copyright 2013 Twitter, Inc. | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package com.twitter.summingbird.online | ||
|
||
import org.scalacheck._ | ||
import Gen._ | ||
import Arbitrary._ | ||
import org.scalacheck.Prop._ | ||
|
||
import com.twitter.util.{Return, Throw, Future, Try} | ||
|
||
object QueueChannelLaws extends Properties("Channel") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great work with all the tests, the only one I don't see and am not sure matters is just ordering of put to poll There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will add. |
||
|
||
property("Putting into a BoundedQueue gets size right") = forAll { (items: List[String]) => | ||
val q = Channel[String]() | ||
q.putAll(items) | ||
q.size == items.size | ||
} | ||
property("not spill if capacity is enough") = forAll { (items: List[Int]) => | ||
val q = Channel[Int]() | ||
q.putAll(items) | ||
q.trimTo(items.size).size == 0 | ||
} | ||
property("Work with indepent additions") = forAll { (items: List[Int]) => | ||
val q = Channel[Int]() | ||
items.map(q.put(_)) == (1 to items.size).toList | ||
} | ||
property("spill all with zero capacity") = forAll { (items: List[Int]) => | ||
val q = Channel[Int]() | ||
q.putAll(items) | ||
q.trimTo(0) == items | ||
} | ||
property("Channel works with finished futures") = forAll { (items: List[Int]) => | ||
val q = Channel.linkedBlocking[(Int,Try[Int])] | ||
items.foreach { i => q.put((i, Try(i*i))) } | ||
q.foldLeft((0, true)) { case ((cnt, good), (i, ti)) => | ||
ti match { | ||
case Return(ii) => (cnt + 1, good) | ||
case Throw(e) => (cnt + 1, false) | ||
} | ||
} == (items.size, true) | ||
} | ||
property("Channel.linkedNonBlocking works") = forAll { (items: List[Int]) => | ||
val q = Channel.linkedNonBlocking[(Int,Try[Int])] | ||
items.foreach { i => q.put((i, Try(i*i))) } | ||
q.foldLeft((0, true)) { case ((cnt, good), (i, ti)) => | ||
ti match { | ||
case Return(ii) => (cnt + 1, good) | ||
case Throw(e) => (cnt + 1, false) | ||
} | ||
} == (items.size, true) | ||
} | ||
property("Channel foreach works") = forAll { (items: List[Int]) => | ||
// Make sure we can fit everything | ||
val q = Channel.arrayBlocking[(Int,Try[Int])](items.size + 1) | ||
items.foreach { i => q.put((i,Try(i*i))) } | ||
var works = true | ||
q.foreach { case (i, Return(ii)) => | ||
works = works && (ii == i*i) | ||
} | ||
works && (q.size == 0) | ||
} | ||
property("Channel foldLeft works") = forAll { (items: List[Int]) => | ||
// Make sure we can fit everything | ||
val q = Channel.arrayBlocking[(Int,Try[Int])](items.size + 1) | ||
items.foreach { i => q.put((i,Try(i*i))) } | ||
q.foldLeft(true) { case (works, (i, Return(ii))) => | ||
(ii == i*i) | ||
} && (q.size == 0) | ||
} | ||
|
||
property("Channel poll + size is correct") = forAll { (items: List[Int]) => | ||
// Make sure we can fit everything | ||
val q = Channel[Int]() | ||
items.map { i => | ||
q.put(i) | ||
val size = q.size | ||
if(i % 2 == 0) { | ||
// do a poll test | ||
q.poll match { | ||
case None => q.size == 0 | ||
case Some(_) => q.size == (size - 1) | ||
} | ||
} | ||
else true | ||
}.forall(identity) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this now mostly just an abstract Queue class now? not sure the name matches anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.