[
https://issues.apache.org/jira/browse/CASSANDRA-15564?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17050629#comment-17050629
]
Blake Eggleston commented on CASSANDRA-15564:
---------------------------------------------
This is almost there, and shaping up nicely. It's nice to see some attention
being paid to organizing RepairRunnable.
All my remaining feedback is for RepairRunnable, and is almost all minor:
* I think {{unsafeRun}} could be folded into run. If you'd like to keep it
separate, maybe rename it to runMayThrow or something? unsafe* usually implies
we're making some internal function visible for testing.
* {{getColumnFamilies}} can use guava's {{Lists.newArrayList(Iterable<T> i)}}
instead of manually creating the list.
* {{maybeCreateTraceState}}, totally stylistic and up to you, but I personally
like to omit braces for short "if then bail out" type blocks like the one here
and in {{getColumnFamilies}}. Four lines seems like a bit much for simple
things like that, but again, totally up to you.
* {{populateNeighborsAndRanges}}
** the indentation needs to be fixed on line 390
** could this be folded into getNeighborsAndRanges? It doesn't seem like it
needs it's own methods.
* {{tryStoreParentRepairStart}} - the naming convention with methods like
these would be {{maybeStoreParentRepairStart}}, try implies error handling,
which isn't really applicable here
* {{repair}} you can just pass the NeighborsAndRanges object in here
* setting {{creationTimeMillies}} in the ctor is going to change how repair
times are reported in some cases, since there may be a significant delay
between runnable creation and starting in some cases. It would be good to
_also_ report this, which I don't think we do, but we shouldn't combine them,
since they can each reveal different problems.
> Refactor repair coordinator so errors are consistent
> ----------------------------------------------------
>
> Key: CASSANDRA-15564
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15564
> Project: Cassandra
> Issue Type: Sub-task
> Components: Consistency/Repair
> Reporter: David Capwell
> Assignee: David Capwell
> Priority: Normal
> Labels: pull-request-available
> Time Spent: 11h 10m
> Remaining Estimate: 0h
>
> This is to split the change in CASSANDRA-15399 so the refactor is isolated
> out.
> Currently the repair coordinator special cases the exit cases at each call
> site; this makes it so that errors can be inconsistent and there are cases
> where proper complete isn't done (proper notifications, and forgetting to
> update ActiveRepairService).
> [Circle
> CI|https://circleci.com/gh/dcapwell/cassandra/tree/bug%2FrepairCoordinatorJmxConsistency]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
