Change in asterixdb[master]: Introduce MessagingNetworkManager for NC2NC AppMessaging

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

Till Westmann has posted comments on this change.

Change subject: Introduce MessagingNetworkManager for NC2NC AppMessaging
......................................................................


Patch Set 6:

(7 comments)

https://asterix-gerrit.ics.uci.edu/#/c/897/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/MessagingChannelInterfaceFactory.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/MessagingChannelInterfaceFactory.java:

Line 90:                     if (LOGGER.isLoggable(Level.WARNING)) {
Maybe this should just throw an exception? It seems that everywhere we call this method we wither assert that the buffer is not null or we access the buffer without checking (getting an NPE it it ever were null). A NetException might work ..


https://asterix-gerrit.ics.uci.edu/#/c/897/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/MessagingChannelReadInterface.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/MessagingChannelReadInterface.java:

Line 31: public class MessagingChannelReadInterface implements IChannelReadInterface {
It seems that there's a lot of overlap with FullFrameChannelReadInterface. Is there an opportunity for some reuse (in a reasonably generic way - i.e. no hack to exactly support this implementation).


https://asterix-gerrit.ics.uci.edu/#/c/897/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/MessagingChannelWriteInterface.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/MessagingChannelWriteInterface.java:

Line 36: public class MessagingChannelWriteInterface implements IChannelWriteInterface {
It seems that there's a lot of overlap with FullFrameChannelWriteInterface. Is there an opportunity for some reuse (in a reasonably generic way - i.e. no hack to exactly support this implementation).


https://asterix-gerrit.ics.uci.edu/#/c/897/6/asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/NCMessageBroker.java
File asterixdb/asterix-app/src/main/java/org/apache/asterix/messaging/NCMessageBroker.java:

Line 66:     private final MessageDeliveryService msgDeliverySvc;
Do we need to keep this as a member?


Line 288:                     Thread.currentThread().interrupt();
Is there a reason why we shouldn't exit run(), if the thread gets interrupted?


Line 291:                         LOGGER.log(Level.WARNING, "Could not process message", e);
Could we add some info (e.g. the message id) to the warning?


https://asterix-gerrit.ics.uci.edu/#/c/897/6/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IChannelReadInterface.java
File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/comm/IChannelReadInterface.java:

Line 98:     public IBufferFactory getBufferFactor();
a/getBufferFactor/getBufferFactory/


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c0bd7c11c1e78954ebceff49cb274d8073a64bd
Gerrit-PatchSet: 6
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <hu...@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 <bu...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <ba...@gmail.com>
Gerrit-HasComments: Yes