asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yingyi Bu (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Remove ICCContext
Date Sat, 29 Oct 2016 17:12:47 GMT
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 <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message