Change in hyracks[master]: Support Sending Messages Alongside Frame Data

["Till Westmann (Code Review)" <do...@asterixdb.incubator.apache.org>]

Till Westmann has posted comments on this change.

Change subject: Support Sending Messages Alongside Frame Data
......................................................................


Patch Set 3:

(7 comments)

For the first round I've got a few questions at the code level (and not all of them are suggestions :) ).

https://asterix-gerrit.ics.uci.edu/#/c/604/3/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/InsertDeleteOperator.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/InsertDeleteOperator.java:

Line 53:     private final boolean feed;
Why does a Hyracks operator know anything about feeds?

Would there be a more generic way to describe the property?

Also, if we need an additional piece of information, why don't we use the existing annotations that are available on each operator?
(The same question probably applies to bulkload.)


https://asterix-gerrit.ics.uci.edu/#/c/604/3/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/HashPartitionExchangePOperator.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/HashPartitionExchangePOperator.java:

Line 51:     protected INodeDomain domain;
It seems that these could remain private.


https://asterix-gerrit.ics.uci.edu/#/c/604/3/algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/EnforceStructuralPropertiesRule.java
File algebricks/algebricks-rewriter/src/main/java/org/apache/hyracks/algebricks/rewriter/rules/EnforceStructuralPropertiesRule.java:

Line 575:                         if (op instanceof InsertDeleteOperator && ((InsertDeleteOperator) op).isFeed()) {
If we used operator annotations we could also avoid the instanceof and cast here.


https://asterix-gerrit.ics.uci.edu/#/c/604/3/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/context/IHyracksTaskContext.java
File hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/context/IHyracksTaskContext.java:

Line 52:     public Object getObject(String name);
Could we achieve this with the state object of the IOperatorEnvironment?

If we need the methods here, I think that we only should have one setter/getter pair. Right now we have 2 pairs of methods on the interface that are linked in the implementation. So 1 pair seems to be redundant.

If we need the information on this context, what are the trade-offs between adding it here and adding it to the IHyracksCommonContext (and avoiding the other interface changes, e.g. in AsterixDB's ITupleForwarder).


https://asterix-gerrit.ics.uci.edu/#/c/604/3/hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/io/MessagingFrameTupleAppender.java
File hyracks/hyracks-dataflow-common/src/main/java/org/apache/hyracks/dataflow/common/io/MessagingFrameTupleAppender.java:

Line 41:     public MessagingFrameTupleAppender(IFrame frame, IHyracksTaskContext ctx) throws HyracksDataException {
There seems to be no need for this constructor.


https://asterix-gerrit.ics.uci.edu/#/c/604/3/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/MToNPartitioningConnectorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/MToNPartitioningConnectorDescriptor.java:

Line 49:         final PartitionDataWriter hashWriter = new PartitionDataWriter(ctx, nConsumerPartitions, edwFactory, recordDesc,
This variable could be removed.


https://asterix-gerrit.ics.uci.edu/#/c/604/3/hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/MToNPartitioningWithMessageConnectorDescriptor.java
File hyracks/hyracks-dataflow-std/src/main/java/org/apache/hyracks/dataflow/std/connectors/MToNPartitioningWithMessageConnectorDescriptor.java:

Line 42:         final PartitionWithMessageDataWriter hashWriter = new PartitionWithMessageDataWriter(ctx, nConsumerPartitions,
This variable could be removed.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/604
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I56ae8124052c13a52ca42965b8d00e18ecf35a28
Gerrit-PatchSet: 3
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <ba...@gmail.com>
Gerrit-Reviewer: Jenkins <je...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hu...@gmail.com>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <yi...@google.com>
Gerrit-HasComments: Yes