Michael Blow has posted comments on this change. Change subject: Configuration Revamp ......................................................................
Patch Set 30: (27 comments) https://asterix-gerrit.ics.uci.edu/#/c/1487/30/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java: PS30, Line 126: getCCConfig > Does the ICCApplicationEntryPoint know about the config manager? i.e. shouldn't it already, since we had it registerConfigOptions with it, or that it seems wrong that it should? https://asterix-gerrit.ics.uci.edu/#/c/1487/30/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/ClusterStateDefaultParameterTest.java File asterixdb/asterix-app/src/test/java/org/apache/asterix/test/runtime/ClusterStateDefaultParameterTest.java: PS30, Line 68: /node/asterix_nc1/config > How did the HTTP API change here? These are NC options now, reported under the NC config, not [app] options (i.e. global) as before. https://asterix-gerrit.ics.uci.edu/#/c/1487/30/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/CompilerProperties.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/CompilerProperties.java: PS30, Line 21: .*; > Resolve '*' includes? Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/ExternalProperties.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/ExternalProperties.java: PS30, Line 63: CC > Is this correct? No, and neither is NC; same goes for CC above for CC_JAVA_OPTS. These are options only used by asterix-configuration.xml & managix, and are not used in the ncservice world. I've introduced a new section type (NULL) for these. It's possible these should be merged with virtual options. https://asterix-gerrit.ics.uci.edu/#/c/1487/30/asterixdb/asterix-events/src/main/resources/events/node_join/nc_join.sh File asterixdb/asterix-events/src/main/resources/events/node_join/nc_join.sh: PS30, Line 24: > fix WS? Done PS30, Line 25: > fix WS? Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/asterixdb/asterix-events/src/main/resources/events/node_restart/nc_restart.sh File asterixdb/asterix-events/src/main/resources/events/node_restart/nc_restart.sh: PS30, Line 26: > fix WS? Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/algebricks/algebricks-tests/src/test/java/org/apache/hyracks/algebricks/tests/util/AlgebricksHyracksIntegrationUtil.java File hyracks-fullstack/algebricks/algebricks-tests/src/test/java/org/apache/hyracks/algebricks/tests/util/AlgebricksHyracksIntegrationUtil.java: PS30, Line 66: data > line too long? Done PS30, Line 78: data > line too long? Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/config/IOption.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/config/IOption.java: PS30, Line 24: IOption > I think that we need some javadoc here - especially on "hidden" and "virtua Done PS30, Line 52: defaultValue > Not a big fan of default implementations - especially if they get complicat Done PS30, Line 67: usage > Not a big fan of default implementations - especially if they get complicat Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/context/ICCContext.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/context/ICCContext.java: PS30, Line 37: getConfigManager > Do we need to get the config manager from the ICCContext? It seems that the Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/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: PS30, Line 147: aep > Validate that the now necessary AEP is not null? Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/OptionMarker.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/OptionMarker.java: PS30, Line 32: setter > Add an example? Setters is not implemented, doc updated. https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/NodeControllerService.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/NodeControllerService.java: PS30, Line 169: aep > It seems that the application entry point cannot be null anymore. Shoudl we Done Line 245: messagingNetManager = new MessagingNetworkManager(this, ncConfig.messagingListenAddress, ncConfig.messagingListenPort, > Indeed :) Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-control/hyracks-nc-service/src/main/java/org/apache/hyracks/control/nc/service/NCService.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-nc-service/src/main/java/org/apache/hyracks/control/nc/service/NCService.java: PS30, Line 195: printStackTrace > Should this stay in here or should we add an issue to remove it? Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/NCApplicationEntryPoint.java File hyracks-fullstack/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/NCApplicationEntryPoint.java: PS30, Line 58: getNCConfig > A little confused about the scopes here. The NCConfig seems to be about the Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/integration/AbstractIntegrationTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/integration/AbstractIntegrationTest.java: PS30, Line 107: data > line too long? Done PS30, Line 118: data > line too long? Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/integration/PredistributedJobsTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/integration/PredistributedJobsTest.java: PS30, Line 80: data > line too long? Done PS30, Line 92: data > line too long? Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCConnectionManager.java File hyracks-fullstack/hyracks/hyracks-ipc/src/main/java/org/apache/hyracks/ipc/impl/IPCConnectionManager.java: PS30, Line 326: SEVERE > Why is this severe? Don't we intend to continue? (I guess we should agree o I think that this was getting swallowed, but I think that had more to do with the fact that the loggers weren't configured, and out of frustration, I escalated to SEVERE. I do think we have an issue here, in that we retry forever when we fail to process the message... https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-server/docs/README File hyracks-fullstack/hyracks/hyracks-server/docs/README: PS30, Line 22: > alignment seems to be broken Done https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-server/src/main/java/org/apache/hyracks/server/process/HyracksCCProcess.java File hyracks-fullstack/hyracks/hyracks-server/src/main/java/org/apache/hyracks/server/process/HyracksCCProcess.java: Line 44: //cList.add("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005"); > Just revert the file? This is pretty useful- maybe instead add a flag so that it's not commented out code? https://asterix-gerrit.ics.uci.edu/#/c/1487/30/hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/server/test/NCServiceIT.java File hyracks-fullstack/hyracks/hyracks-server/src/test/java/org/apache/hyracks/server/test/NCServiceIT.java: PS30, Line 80: TODO > File an issue and revert the file? Reverted the comment, the keeping change to eliminate the unused imports. -- To view, visit https://asterix-gerrit.ics.uci.edu/1487 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95b7e0bd4538ef42817c8826e76412150074b754 Gerrit-PatchSet: 30 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-HasComments: Yes
