madrob commented on a change in pull request #2148:
URL: https://github.com/apache/lucene-solr/pull/2148#discussion_r552865278
##########
File path:
solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java
##########
@@ -564,12 +539,17 @@ public boolean isPreOp() {
}
/**
+ * This method should compute the set of ZK operations for a given action
+ * for instance, a state change may result in
Review comment:
I think this sentence ends mid-way.
##########
File path:
solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java
##########
@@ -352,111 +349,103 @@ public int hashCode() {
* Do not directly manipulate the per replica states as it can become
difficult to debug them
*
*/
- public static abstract class WriteOps {
+ public static class WriteOps {
Review comment:
I still think this class should be moved out to its own file. There is a
LOT going on in here and it is deserving of a separate compilation unit.
##########
File path:
solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java
##########
@@ -465,75 +454,61 @@ public static WriteOps flipLeader(Set<String>
allReplicas, String next, PerRepli
* @param replica name of the replica to be deleted
*/
public static WriteOps deleteReplica(String replica, PerReplicaStates rs) {
- return new WriteOps() {
- @Override
- protected List<Operation> refresh(PerReplicaStates rs) {
- List<Operation> result;
- if (rs == null) {
- result = Collections.emptyList();
- } else {
- State state = rs.get(replica);
- result = addDeleteStaleNodes(new ArrayList<>(), state);
- }
- return result;
+ return new WriteOps(prs -> {
+ List<Operation> result;
+ if (rs == null) {
+ result = Collections.emptyList();
+ } else {
+ State state = rs.get(replica);
+ result = addDeleteStaleNodes(new ArrayList<>(), state);
}
- }.init(rs);
+ return result;
+ }).init(rs);
}
public static WriteOps addReplica(String replica, Replica.State state,
boolean isLeader, PerReplicaStates rs) {
- return new WriteOps() {
- @Override
- protected List<Operation> refresh(PerReplicaStates rs) {
- return singletonList(new Operation(Operation.Type.ADD,
- new State(replica, state, isLeader, 0)));
- }
- }.init(rs);
+ return new WriteOps(perReplicaStates -> singletonList(new
Operation(Operation.Type.ADD,
+ new State(replica, state, isLeader, 0)))).init(rs);
}
/**
* mark a bunch of replicas as DOWN
*/
public static WriteOps downReplicas(List<String> replicas,
PerReplicaStates rs) {
- return new WriteOps() {
- @Override
- List<Operation> refresh(PerReplicaStates rs) {
- List<Operation> ops = new ArrayList<>();
- for (String replica : replicas) {
- State r = rs.get(replica);
- if (r != null) {
- if (r.state == Replica.State.DOWN && !r.isLeader) continue;
- ops.add(new Operation(Operation.Type.ADD, new State(replica,
Replica.State.DOWN, Boolean.FALSE, r.version + 1)));
- addDeleteStaleNodes(ops, r);
- } else {
- ops.add(new Operation(Operation.Type.ADD, new State(replica,
Replica.State.DOWN, Boolean.FALSE, 0)));
- }
- }
- if (log.isDebugEnabled()) {
- log.debug("for coll: {} down replicas {}, ops {}", rs, replicas,
ops);
+ return new WriteOps(prs -> {
+ List<Operation> operations = new ArrayList<>();
+ for (String replica : replicas) {
+ State r = rs.get(replica);
+ if (r != null) {
+ if (r.state == Replica.State.DOWN && !r.isLeader) continue;
+ operations.add(new Operation(Operation.Type.ADD, new
State(replica, Replica.State.DOWN, Boolean.FALSE, r.version + 1)));
+ addDeleteStaleNodes(operations, r);
+ } else {
+ operations.add(new Operation(Operation.Type.ADD, new
State(replica, Replica.State.DOWN, Boolean.FALSE, 0)));
}
- return ops;
}
- }.init(rs);
+ if (log.isDebugEnabled()) {
+ log.debug("for coll: {} down replicas {}, ops {}", rs, replicas,
operations);
+ }
+ return operations;
+ }).init(rs);
}
/**
- * Just creates and deletes a summy entry so that the {@link
Stat#getCversion()} of states.json
+ * Just creates and deletes a dummy entry so that the {@link
Stat#getCversion()} of states.json
* is updated
*/
public static WriteOps touchChildren() {
- WriteOps result = new WriteOps() {
- @Override
- List<Operation> refresh(PerReplicaStates rs) {
- List<Operation> ops = new ArrayList<>();
- State st = new State(".dummy." + System.nanoTime(),
Replica.State.DOWN, Boolean.FALSE, 0);
- ops.add(new Operation(Operation.Type.ADD, st));
- ops.add(new Operation(Operation.Type.DELETE, st));
- if (log.isDebugEnabled()) {
- log.debug("touchChildren {}", ops);
- }
- return ops;
+ WriteOps result = new WriteOps(prs -> {
+ List<Operation> operations = new ArrayList<>();
Review comment:
nit: set initial size = 2
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
