> On Aug. 28, 2013, 9:39 p.m., Sriram Subramanian wrote: > > I don't really like the fact that the chooser's implementation depends on > > how it gets called. That seems fragile. > > Jay Kreps wrote: > In fairness that contract is documented in the interface so changing it > is effectively changing the interface. I think it is actually a safe > assumption because allowing prioritization of more than one message from a > given stream breaks the linear processing requirement of logs and offsets. > That said, I think maybe at heart your complaint was similar to mine which is > just that update/choose is an unusual way to express an ordering. But I can't > think of a better one that is fully general (the priority thing was as close > as we got and as you pointed out that straight-jackets you into a heap and > isn't the most natural for other cases--e.g. doing round-robin by using a > continually decremented counter is clever but weird, right?).
Three thoughts: 1. Do nothing. 2. Make the RoundRobinChooser protect itself (throw an exception), to guarantee only one message is received at a time. 3. Implement a RoundRobinChooser that works if multiple messages from a single SSP are update()'d even though none have been returned yet. The third option is essentially dangerous and wrong. Doing this can result in re-ordering messages within a partition, in the general case. In the specific RoundRobinChooser case, it should be fine, since round robin'ing is just FIFO'ing within a single stream partition, and no re-ordering is taking place. Still, I don't like supporting something that is broken behavior on the part of the container. I'm going to leave it how it is. - Chris ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13725/#review25688 ----------------------------------------------------------- On Aug. 28, 2013, 6:23 p.m., Chris Riccomini wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13725/ > ----------------------------------------------------------- > > (Updated Aug. 28, 2013, 6:23 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > adding docs for message chooser. swiching round robin chooser back to a queue. > > > missed license in message chooser factory > > > add apache licensing > > > samza container was using message chooser, not message chooser factory. fixed. > > > add stream chooser test. update stream chooser to invert priority due to bug. > > > add round robin test. fix compile error in round robin chooser. > > > add priority chooser test. fix bug in priority chooser that was reversing > ordering. > > > adding stream chooser. adding message chooser factory. > > > adding priority chooser. moving default chooser to round robin chooser. > adding config for chooser > > > Diffs > ----- > > docs/learn/documentation/0.7.0/container/streams.md > e755789407b294e02b399e71ba684c1d6dc314c6 > samza-api/src/main/java/org/apache/samza/system/MessageChooser.java > 306b2902303c72f3d7a3eb313f55d7e88d21e00d > samza-api/src/main/java/org/apache/samza/system/MessageChooserFactory.java > PRE-CREATION > samza-api/src/main/java/org/apache/samza/system/PriorityChooser.java > PRE-CREATION > samza-api/src/test/java/org/apache/samza/system/TestPriorityChooser.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/config/StreamChooserConfig.scala > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/config/TaskConfig.scala > 0c742d83c2f60d2448a79376677713a1ff0b11ec > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala > 2d2efdd14c7680c29aad5f2a98349e2fc57cf9fe > samza-core/src/main/scala/org/apache/samza/system/DefaultChooser.scala > 5a72e7a3bfba0f06a5a98c6ba26865800d7780b9 > samza-core/src/main/scala/org/apache/samza/system/StreamChooser.scala > PRE-CREATION > > samza-core/src/test/scala/org/apache/samza/system/TestRoundRobinChooser.scala > PRE-CREATION > samza-core/src/test/scala/org/apache/samza/system/TestStreamChooser.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/13725/diff/ > > > Testing > ------- > > > Thanks, > > Chris Riccomini > >
