asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Murtadha Hubail (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
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:

(12 comments)

@Abdullah,
I addressed your comments and also updated the fully descriptive test files names that you
criticised :)

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.
Done


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?
Done. I also added comments on the possible states in AsterixClusterProperties at each stage
of the plan lifecycle.


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.
Done


Line 265:         HttpMethodBase method = new GetMethod(url);;
> remove additional semi-colon
Done


Line 527:                             if (queryCount == expectedResultFileCtxs.size() - 1)
{
> https://asterix-gerrit.ics.uci.edu/#/c/625/12/asterix-common/src/test/java/
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 
Done


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
Done


Line 584:         } else {
> fix bug. else if failed, then revert or something
Done


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??
Done


Line 300:                         //if the component belong to a shit
> Fix comment
Done


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?
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 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 <hubailmor@gmail.com>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Murtadha Hubail <hubailmor@gmail.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message