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 <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
