abdullah alamoudi has posted comments on this change.

Change subject: Introduce NC to NC Messaging
......................................................................


Patch Set 1:

(32 comments)

https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java
File 
asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java:

Line 89:             }
> revert this file
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplicationEntryPoint.java
File 
asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplicationEntryPoint.java:

Line 117:             systemState = recoveryMgr.getSystemState();
> revert this file
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-app/src/main/java/org/apache/asterix/messaging/NCMessageDecoder.java
File 
asterix-app/src/main/java/org/apache/asterix/messaging/NCMessageDecoder.java:

Line 31:         System.err.println("NCMessageDecoder: decode called");
> Use logger.
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-app/src/main/java/org/apache/asterix/messaging/NCMessagingClientHandler.java
File 
asterix-app/src/main/java/org/apache/asterix/messaging/NCMessagingClientHandler.java:

Line 62:         cause.printStackTrace();
> remove printStackTrace()
Done


Line 62:         cause.printStackTrace();
> Use logger to print the exception message
Done


Line 77:             if (future.isSuccess()) {
> use try-finally and put future.channel().close() in the finally block.
Done


Line 80:                 future.cause().printStackTrace();
> remove printStackTrace()
Done


Line 80:                 future.cause().printStackTrace();
> Use logger to print the exception message
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-app/src/main/java/org/apache/asterix/messaging/NCMessagingHandler.java
File 
asterix-app/src/main/java/org/apache/asterix/messaging/NCMessagingHandler.java:

Line 41:         System.err.println("channelRead Called");
> remove println
Done


Line 47:             System.out.println(buf.readInt());
> remove println
Done


Line 54:         cause.printStackTrace();
> remove printStackTrace
Done


Line 54:         cause.printStackTrace();
> Use logger to print the exception message
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-app/src/main/java/org/apache/asterix/messaging/NCMessagingServer.java
File 
asterix-app/src/main/java/org/apache/asterix/messaging/NCMessagingServer.java:

Line 32:     private int port;
> final
Done


Line 33:     private ChannelInboundHandlerAdapter handler = new 
NCMessagingHandler();
> final
Done


Line 34:     private ChannelFuture f;
> rename to channelFuture
Done


Line 64:                     // In this example, this does not happen, but you 
can do that to gracefully
> Remove comment after first line
Done


Line 68:                     // Do something
> remove comment
Done


Line 69:                     th.printStackTrace();
> remove printStackTrace()
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-app/src/main/resources/cluster.xml
File asterix-app/src/main/resources/cluster.xml:

Line 35:         <nc_messaging_port>4501</nc_messaging_port>
> rename to messaging_port
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-common/src/main/resources/schema/asterix-conf.xsd
File asterix-common/src/main/resources/schema/asterix-conf.xsd:

Line 60:         name="nc_messaging_port"
> rename to messagingPort (not that I like it, but just to be consistent with
Done


Line 62:         
> ws
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-events/src/main/java/org/apache/asterix/event/management/EventTask.java
File 
asterix-events/src/main/java/org/apache/asterix/event/management/EventTask.java:

Line 27: 
> revert this file
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-events/src/main/java/org/apache/asterix/event/service/AsterixEventServiceUtil.java
File 
asterix-events/src/main/java/org/apache/asterix/event/service/AsterixEventServiceUtil.java:

Line 281:         String nodeFeedPort;
> rename to nodeMessagingPort
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-external-data/pom.xml
File asterix-external-data/pom.xml:

Line 291:             <groupId>io.netty</groupId>
> open a JIRA to add the library license if needed
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedChannelManager.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedChannelManager.java:

Line 19: package org.apache.asterix.external.feed.management;
> remove this file
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedManager.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/management/FeedManager.java:

Line 48:     private FeedChannelManager feedChannelManager;
> remove variable and its usage.
Done


Line 90:             e.printStackTrace();
> why catch this exception? shouldn't it be illegalState to prevent the NC fr
Done


Line 90:             e.printStackTrace();
> remove printStackTrace()
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-external-data/src/main/java/org/apache/asterix/external/feed/message/FeedMessageHandler.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/message/FeedMessageHandler.java:

Line 20: 
> this class is not needed anymore, right?
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-external-data/src/main/java/org/apache/asterix/external/feed/message/FeedMessageServer.java
File 
asterix-external-data/src/main/java/org/apache/asterix/external/feed/message/FeedMessageServer.java:

Line 29: 
> this class is not needed anymore, right?
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-installer/src/main/java/org/apache/asterix/installer/command/ValidateCommand.java
File 
asterix-installer/src/main/java/org/apache/asterix/installer/command/ValidateCommand.java:

Line 213:             }
> you need to add the check here for messaging port if it isn't defined on bo
Done


https://asterix-gerrit.ics.uci.edu/#/c/657/1/asterix-yarn/src/main/java/org/apache/asterix/aoya/AsterixYARNClient.java
File asterix-yarn/src/main/java/org/apache/asterix/aoya/AsterixYARNClient.java:

Line 1361:             stores.add(new Store(node.getId(), "4501", 
nodeStores.toString()));
> you need to update the yarn cluster file and pass the correct port.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I85bd5ea8ad5b50a6cfb25088d8853f26902799f9
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[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

Reply via email to