[jira] [Commented] (CASSANDRA-16113) Consolidate dead nodes check in force repair

["Blake Eggleston (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-16113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17209819#comment-17209819 ] 

Blake Eggleston commented on CASSANDRA-16113:
---------------------------------------------

Just a few minor naming type stuff:

* RepairRunnable.java
	** LiveNeighorsAndRanges
		*** I think naming would make more sense if we changed the naming around for this a bit
			**** Rename class back to `NeighorsAndRanges`, since it can contain dead neighbors if the force flag wasn't set.
			**** Rename `allNeighbors` to `participants`, so we're not implying liveness or completeness
			**** Rename `force` to `deadParticipantsExcluded` or something, to indicate the contents of `participants`
			**** WDYT?
	** `filterCommonRanges`
		*** I'd reword the start of the new log message a bit. Something like "Skipping forced repair for ...", which is a bit clearer about what's happening and why
		*** you only need `Sets.*difference*(commonRange.endpoints, endpoints)` when computing skipped replicas, since trans endpoints are a subset of all endpoints

* DistributedRepairUtils.java
	** let's add a null check for `moreSuccessCritera` at the top of `assertParentRepairSuccess` so this fails quickly in the case of a null arg

> Consolidate dead nodes check in force repair
> --------------------------------------------
>
>                 Key: CASSANDRA-16113
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-16113
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Local/Other
>            Reporter: Yifan Cai
>            Assignee: Yifan Cai
>            Priority: Normal
>             Fix For: 4.0-beta
>
>          Time Spent: 4h 10m
>  Remaining Estimate: 0h
>
> The check for dead nodes during force repair is duplicated in the normal and incremental repair. We could consolidate those 2 checks to make the code more dry. 
> The check should throw a more meaningful error message to indicate that all neighbor nodes are down, instead of "java.lang.IllegalArgumentException: Endpoints can not be empty"



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org