asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Murtadha Hubail (Code Review)" <>
Subject Change in asterixdb[master]: Asterix NCs Failback Support
Date Wed, 17 Feb 2016 06:18:25 GMT
Murtadha Hubail has posted comments on this change.

Change subject: Asterix NCs Failback Support

Patch Set 4:


I addressed your comments and also updated the fully descriptive test files names that you
criticised :)
File asterix-app/src/main/java/org/apache/asterix/api/common/

Line 183:         PersistentLocalResourceRepository persistentLocalResourceRepository = (PersistentLocalResourceRepository)
> instead of down casting, why not change localResourceRepository type.
File asterix-common/src/main/java/org/apache/asterix/common/replication/

Line 31:     public enum FailbackPlanState {
> Can you add comments to make sure the state transition is clear?
Done. I also added comments on the possible states in AsterixClusterProperties at each stage
of the plan lifecycle.
File asterix-common/src/test/java/org/apache/asterix/test/aql/

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)
As we discussed, I remove this and added it to the finally block similar to your change.

Line 629:                                         "http://" + host + ":" + port + "/admin/cluster");
> get the url from the servlet definition so if it changes, we don't need to 
File asterix-om/src/main/java/org/apache/asterix/om/util/

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
File asterix-replication/src/main/java/org/apache/asterix/replication/management/

Line 277:                 break;
> doesn't look nice. find a better way??

Line 300:                         //if the component belong to a shit
> Fix comment
File asterix-replication/src/main/java/org/apache/asterix/replication/recovery/

Line 72:         int maxRecoveryAttempts = 5;
> get from a configuration file?
I changed it to be read from AsterixReplicationProperties but currently it is just a constant,
but it can be read from a config file later.

Line 251:     public void startFailbackProcess() {
> get from conf?
I changed it to be read from AsterixReplicationProperties but currently it is just a constant,
but it can be read from a config file later.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Id17819542d6b9c4e32647e64737c4a467b630f24
Gerrit-PatchSet: 4
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <>
Gerrit-Reviewer: Ian Maxon <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Murtadha Hubail <>
Gerrit-Reviewer: Till Westmann <>
Gerrit-Reviewer: abdullah alamoudi <>
Gerrit-HasComments: Yes

View raw message