Murtadha Hubail has posted comments on this change. Change subject: Introduce MessagingNetworkManager for NC2NC AppMessaging ......................................................................
Patch Set 6: (10 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 Done. Since the IBufferFactory interface is in Hyracks API common project, I made it throw HyracksDataException. If you think a NetException is more appropriate, I will gladly change it. 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. I introduced an abstract implementation (AbstractChannelReadInterface) and moved all common parts there. 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. I introduced an abstract implementation (AbstractChannelWriteInterface) and moved all common parts there. Line 44: private final ICloseableBufferAcceptor fullBufferAcceptor = new ICloseableBufferAcceptor() { > MAJOR SonarQube violation: Moved as a private class. 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? No, at least not now. I moved it inside the constructor. Line 288: Thread.currentThread().interrupt(); > Is there a reason why we shouldn't exit run(), if the thread gets interrupt Please see my detailed response on this in patch set 4. Line 291: LOGGER.log(Level.WARNING, "Could not process message", e); > Could we add some info (e.g. the message id) to the warning? Done 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/ Done https://asterix-gerrit.ics.uci.edu/#/c/897/6/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/NetException.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/NetException.java: Line 24: public NetException() { > MAJOR SonarQube violation: Done https://asterix-gerrit.ics.uci.edu/#/c/897/6/hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestNCApplicationContext.java File hyracks-fullstack/hyracks/hyracks-test-support/src/main/java/org/apache/hyracks/test/support/TestNCApplicationContext.java: Line 151: public void setMessagingChannelInterfaceFactory(IChannelInterfaceFactory interfaceFactory) { > MAJOR SonarQube violation: Done -- 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 <hubail...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Murtadha Hubail <hubail...@gmail.com> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
