samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yi Pan \(Data Infrastructure\)" <yi...@linkedin.com>
Subject Re: Review Request 47835: SAMZA-914: Initial draft for Java programming APIs on operators supporting DAGs
Date Tue, 04 Oct 2016 00:27:42 GMT


> 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)
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message