Yingyi Bu has posted comments on this change.

Change subject: Remove ICCContext
......................................................................


Patch Set 4:

(3 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1322/4/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/util/RuntimeUtils.java
File 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/util/RuntimeUtils.java:

Line 58:         map.putAll(((ClusterControllerService) 
AsterixAppContextInfo.INSTANCE.getCCApplicationContext()
Still cast here.


https://asterix-gerrit.ics.uci.edu/#/c/1322/4/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/service/IClusterControllerService.java
File 
hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/service/IClusterControllerService.java:

Line 39:     Map<InetAddress, Set<String>> getIpAddressNodeNameMap();
Why do we need this new interface?

The only difference with ICCContext is that now you can inline 
clustercontrolerinfo and ipaddresses into the ClusterControllerService.

I don't think it's a good idea to make everything inlined into 
ClusterControllerService, because

(1) The class is already large.

(2) Often times, you only need to expose the necessary information, i.e., a 
facet to a client. In this way, a client won't be able to call other 10+ public 
methods in ClusterControllerService, which is dangerous.  Maybe it's not fully 
leveraged in the current codebase, but I don't think the design is bad.


https://asterix-gerrit.ics.uci.edu/#/c/1322/4/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java
File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java:

Line 357:         return ipAddressNodeNameMap;
How do you handle dynamic cluster membership now?

This seems to always return an empty map?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6f6a769741f14e91bcd4b970b4a022c0a453d380
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to