> On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStream.java, > > line 505 > > <https://reviews.apache.org/r/47835/diff/13-17/?file=1487016#file1487016line505> > > > > Where is this used? I couldn't find it on any of the 3 pages of the > > review.
I realized one use case of it during the review: consider ACG re-partition job that consumes 100+ topic partitions but all send to the same output. This is a typical merge case. Does it make sense? > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/TriggerBuilder.java, > > line 65 > > <https://reviews.apache.org/r/47835/diff/17/?file=1514365#file1514365line65> > > > > nit: I'd replace "early" with either "primary" or "regular". > > > > Early trigger contrasts late trigger with opposing terminology, but in > > terms of semantics, we really have a primary trigger, which is expected to > > cover the majority of the messages and then the late trigger to handle late > > arrivals. In that context, "early" doesn't make much sense because it > > doesn't sound like the normal case. > > > > If that^ understanding is correct, I'd suggest a rename. The term is borrowed from Dataflow. It is better to stay w/ the same name w/ the origin, in my opinion, if we adopt the concept from the origin. > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/TriggerBuilder.java, > > line 139 > > <https://reviews.apache.org/r/47835/diff/17/?file=1514365#file1514365line139> > > > > This is essentially the same as "addTimeoutSinceFirstMessage" with a > > custom event time function, right? > > > > Any other differences that I'm not seeing? > > > > No action suggested, just making sure I understand. Yes. > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/TriggerBuilder.java, > > line 213 > > <https://reviews.apache.org/r/47835/diff/17/?file=1514365#file1514365line213> > > > > Surprised to see these default implementations using system time rather > > than event time. Is it just because it's easier to ensure that system time > > exists and is valid? This is not default implementation of "event time". This is specifically for system timeout trigger. Any event time based trigger is in earlyTriggerOnEventTime() > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/TriggerBuilder.java, > > line 223 > > <https://reviews.apache.org/r/47835/diff/17/?file=1514365#file1514365line223> > > > > Why would one put a size limit in a late trigger rather than an early > > trigger? You don't want the late trigger to occur for each and every late arrivals either. This provides a way to suppress the late triggers. > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/WindowState.java, > > line 47 > > <https://reviews.apache.org/r/47835/diff/17/?file=1514366#file1514366line47> > > > > Why the terminology change? Here it's "earliest" and above it's "first" Because this is talking about two very different characteristics: first indicate arrival order, while earliest is much more explicit regarding to temporal order in event time. > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/Windows.java, > > line 38 > > <https://reviews.apache.org/r/47835/diff/17/?file=1514367#file1514367line38> > > > > Add a javadoc recommending a reboot if this class fails. > > > > Also, where's the "Start" button? > > > > :-) LoL. > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/Windows.java, > > line 95 > > <https://reviews.apache.org/r/47835/diff/17/?file=1514367#file1514367line95> > > > > What's the advantage of building the trigger here rather than before > > invoking setTriggers()? This is to hide the Trigger class from the user API. > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/WindowOutput.java, > > line 30 > > <https://reviews.apache.org/r/47835/diff/17/?file=1514372#file1514372line30> > > > > This doesn't seem to add anything to Message. Is it just a placeholder > > in case we want to add something to window outputs and not messages? (For > > example, perhaps information about the trigger that fired.) > > > > Is it the only implementation of Message? Yes. It is a placeholder. I only added this as an implementation of Message since we are focusing on window operator implementation now. > On Oct. 4, 2016, 12:10 a.m., Jake Maes wrote: > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStream.java, > > line 487 > > <https://reviews.apache.org/r/47835/diff/13-17/?file=1487016#file1487016line487> > > > > nit: s/Ds/Ms > > > > There are a few instances Good catch! I have fixed it. Thanks! - Yi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47835/#review151242 ----------------------------------------------------------- On Sept. 29, 2016, 2:05 a.m., Yi Pan (Data Infrastructure) wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47835/ > ----------------------------------------------------------- > > (Updated Sept. 29, 2016, 2:05 a.m.) > > > Review request for samza, Boris Shkolnik, Chris Pettitt, Chinmay Soman, Jake > Maes, Navina Ramesh, Jagadish Venkatraman, and Xinyu Liu. > > > Bugs: SAMZA-914 > https://issues.apache.org/jira/browse/SAMZA-914 > > > Repository: samza > > > Description > ------- > > SAMZA-914: initial draft of operator programming API. Design doc attached to > SAMZA-914: > https://issues.apache.org/jira/secure/attachment/12821524/SAMZA-914_%20operator%20Java%20programming%20API%20-%20Google%20Docs.pdf > > > Diffs > ----- > > build.gradle 16facbbf4dff378c561461786ff186bd9e0000ed > gradle/dependency-versions.gradle 52e25aa53a1edc85d478b48898621b26508ad4bb > samza-api/src/test/java/org/apache/samza/config/TestConfig.java > 5d066c5867e9df9e94e60bde825dedf10703b399 > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStream.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/MessageStreams.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/TriggerBuilder.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/WindowState.java > PRE-CREATION > samza-operator/src/main/java/org/apache/samza/operators/api/Windows.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/data/Message.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/Operators.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/Trigger.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/WindowFn.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/api/internal/WindowOutput.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/ChainedOperators.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/OperatorImpl.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/ProcessorContext.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/SimpleOperatorImpl.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/StateStoreImpl.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/operators/impl/window/SessionWindowImpl.java > PRE-CREATION > > samza-operator/src/main/java/org/apache/samza/task/StreamOperatorAdaptorTask.java > PRE-CREATION > samza-operator/src/main/java/org/apache/samza/task/StreamOperatorTask.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/api/TestTriggerBuilder.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/api/TestWindows.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/api/data/TestIncomingSystemMessage.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/api/data/TestLongOffset.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/api/internal/TestOperators.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/api/internal/TestTrigger.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestOperatorImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestOutputMessage.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestProcessorContext.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestSimpleOperatorImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestSinkOperatorImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/TestStateStoreImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/operators/impl/window/TestSessionWindowImpl.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/task/AssembleCallGraphTask.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/task/BroadcastOperatorTask.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/task/InputAvroSystemMessage.java > PRE-CREATION > samza-operator/src/test/java/org/apache/samza/task/JoinOperatorTask.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/task/TestStreamOperatorAdaptorTask.java > PRE-CREATION > > samza-operator/src/test/java/org/apache/samza/task/TestStreamOperatorTasks.java > PRE-CREATION > samza-operator/src/test/java/org/apache/samza/task/WindowOperatorTask.java > PRE-CREATION > > samza-sql-calcite/src/test/java/org/apache/samza/sql/calcite/schema/TestAvroSchemaConverter.java > PRE-CREATION > samza-sql-core/README.md PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/Data.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/EntityName.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/Relation.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/Schema.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/Stream.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/Table.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/api/data/Tuple.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/Operator.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorCallback.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorRouter.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/OperatorSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/SimpleOperator.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/api/operators/SqlOperatorFactory.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/data/IncomingMessageTuple.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/data/avro/AvroData.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/sql/data/avro/AvroSchema.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/data/serializers/SqlAvroSerde.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/data/serializers/SqlAvroSerdeFactory.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/data/serializers/SqlStringSerde.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/data/serializers/SqlStringSerdeFactory.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/data/string/StringData.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/data/string/StringSchema.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/NoopOperatorCallback.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorFactoryImpl.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorImpl.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleOperatorSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/factory/SimpleRouter.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/join/StreamStreamJoin.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/join/StreamStreamJoinSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/partition/PartitionOp.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/partition/PartitionSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/window/BoundedTimeWindow.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/window/WindowSpec.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/operators/window/WindowState.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/sql/window/storage/OrderedStoreKey.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/system/sql/LongOffset.java > PRE-CREATION > samza-sql-core/src/main/java/org/apache/samza/system/sql/Offset.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/task/sql/RouterMessageCollector.java > PRE-CREATION > > samza-sql-core/src/main/java/org/apache/samza/task/sql/SimpleMessageCollector.java > PRE-CREATION > > samza-sql-core/src/test/java/org/apache/samza/sql/data/serializers/SqlAvroSerdeTest.java > PRE-CREATION > > samza-sql-core/src/test/java/org/apache/samza/task/sql/RandomWindowOperatorTask.java > PRE-CREATION > samza-sql-core/src/test/java/org/apache/samza/task/sql/StreamSqlTask.java > PRE-CREATION > > samza-sql-core/src/test/java/org/apache/samza/task/sql/UserCallbacksSqlTask.java > PRE-CREATION > settings.gradle 4c1aa107a11d413777e69bc4e48847b811aff7d2 > > Diff: https://reviews.apache.org/r/47835/diff/ > > > Testing > ------- > > ./gradlew clean build > > > Thanks, > > Yi Pan (Data Infrastructure) > >
