abdullah alamoudi has posted comments on this change. Change subject: Asterix NCs Failback Support ......................................................................
Patch Set 4: (12 comments) a few comments https://asterix-gerrit.ics.uci.edu/#/c/613/4/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixAppRuntimeContext.java File asterix-app/src/main/java/org/apache/asterix/api/common/AsterixAppRuntimeContext.java: Line 183: PersistentLocalResourceRepository persistentLocalResourceRepository = (PersistentLocalResourceRepository) localResourceRepository; instead of down casting, why not change localResourceRepository type. https://asterix-gerrit.ics.uci.edu/#/c/613/4/asterix-common/src/main/java/org/apache/asterix/common/replication/NodeFailbackPlan.java File asterix-common/src/main/java/org/apache/asterix/common/replication/NodeFailbackPlan.java: Line 31: public enum FailbackPlanState { Can you add comments to make sure the state transition is clear? https://asterix-gerrit.ics.uci.edu/#/c/613/4/asterix-common/src/test/java/org/apache/asterix/test/aql/TestExecutor.java File asterix-common/src/test/java/org/apache/asterix/test/aql/TestExecutor.java: Line 223: System.out.println("Repinse " + errorBody); Fix as you see fit. Line 265: HttpMethodBase method = new GetMethod(url);; remove additional semi-colon Line 527: if (queryCount == expectedResultFileCtxs.size() - 1) { https://asterix-gerrit.ics.uci.edu/#/c/625/12/asterix-common/src/test/java/org/apache/asterix/test/aql/TestExecutor.java Line 629: "http://"; + host + ":" + port + "/admin/cluster"); get the url from the servlet definition so if it changes, we don't need to update it here? https://asterix-gerrit.ics.uci.edu/#/c/613/4/asterix-om/src/main/java/org/apache/asterix/om/util/AsterixClusterProperties.java File asterix-om/src/main/java/org/apache/asterix/om/util/AsterixClusterProperties.java: Line 214: state = ClusterState.ACTIVE; change to while not 0. get rid of recursion Line 584: } else { fix bug. else if failed, then revert or something https://asterix-gerrit.ics.uci.edu/#/c/613/4/asterix-replication/src/main/java/org/apache/asterix/replication/management/ReplicationManager.java File asterix-replication/src/main/java/org/apache/asterix/replication/management/ReplicationManager.java: Line 277: break; doesn't look nice. find a better way?? Line 300: //if the component belong to a shit Fix comment https://asterix-gerrit.ics.uci.edu/#/c/613/4/asterix-replication/src/main/java/org/apache/asterix/replication/recovery/RemoteRecoveryManager.java File asterix-replication/src/main/java/org/apache/asterix/replication/recovery/RemoteRecoveryManager.java: Line 72: int maxRecoveryAttempts = 5; get from a configuration file? Line 251: public void startFailbackProcess() { get from conf? -- To view, visit https://asterix-gerrit.ics.uci.edu/613 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id17819542d6b9c4e32647e64737c4a467b630f24 Gerrit-PatchSet: 4 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
