Add force option and warning for overlapping repair schedules #1099
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1099 +/- ##
==========================================
- Coverage 74.41% 70.16% -4.26%
==========================================
Files 133 133
Lines 9724 9786 +62
Branches 1004 1018 +14
==========================================
- Hits 7236 6866 -370
- Misses 1895 2324 +429
- Partials 593 596 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Awesome work @emerkle826! I love that you went the extra mile and handled the React changes 👏
There are still some combinations that don't work as expected. If I create a full repair schedule on the whole keyspace, and then create an incremental schedule on a specific table of that keyspace, after clicking "Force" I'm getting the following exception in the log:
ERROR [2021-07-02 10:01:25,562] [dw-206 - POST /repair_schedule?clusterName=3-11-6&keyspace=reaper_db&owner=Alex&scheduleTriggerTime=2021-07-02T07%3A49&scheduleDaysBetween=1&tables=running_reapers&repairParallelism=PARALLEL&incrementalRepair=true&repairThreadCount=1&force=true] i.d.j.e.LoggingExceptionMapper - Error handling a request: b12d341e7cad467c
java.lang.IllegalArgumentException: unit conflicts with existing in 3-11-6:reaper_db
at com.google.common.base.Preconditions.checkArgument(Preconditions.java:135)
at io.cassandrareaper.service.RepairUnitService.createRepairUnit(RepairUnitService.java:134)
at io.cassandrareaper.service.RepairUnitService.getOrCreateRepairUnit(RepairUnitService.java:72)
at io.cassandrareaper.resources.RepairScheduleResource.addRepairSchedule(RepairScheduleResource.java:275)
at io.cassandrareaper.resources.RepairScheduleResource.addRepairSchedule(RepairScheduleResource.java:215)
at sun.reflect.GeneratedMethodAccessor48.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
In the comments I also mention a javascript exception that's triggering when schedule responses that are undefined.
One last issue is that forcing the creation will allow to create multiple fully identical schedules:
src/ui/app/jsx/repair-form.jsx
Outdated
| <p>For Cassandra 4.0 and later, you can force creating this schedule by clicking the "Force" button below.</p> | ||
| </Modal.Body> | ||
| <Modal.Footer> | ||
| <Button variant="secondary" onClick={this._onClose}>Close</Button> |
There was a problem hiding this comment.
Could this button be named "Cancel" instead of "Close"? It would make it more obvious that you're canceling the creation.
src/ui/app/jsx/repair-form.jsx
Outdated
| r => this.setState({addRepairResultMsg: r.responseText}) | ||
| r => this.setState({ | ||
| addRepairResultMsg: null, | ||
| addRepairResultStatus: r.status, |
There was a problem hiding this comment.
When I try to overwrite a schedule with the exact same one, it seems like r is undefined which triggers an exception:
We may need a null check here.
There was a problem hiding this comment.
r.status was something I was playing with in testing, but didn't end up using. I need to just remove it here.
I'm losing my mind. I do want status as I'm checking for 409. I'll fix this.
jdonenine
changed the title
|
@adejanovski I'd appreciate another look if you have the time |
emerkle826
force-pushed
the
overlap-warn
branch
3 times, most recently
from
79a9333
to
9c786d6
Compare
There was a problem hiding this comment.
Awesome work @emerkle826 !
With 4.0 coming, this is going to be very helpful.
michaelsembwever
changed the title
|
Removed "K8SSAND-474" from title, as it is downstream. Please keep all Reaper collaboration in the project. |
* Add force option and warning for overlapping repair schedules * [fixup] PR review comments * add tests for force option
This PR adds a "force" option to the add repair schedule API to allow for creating overlapping schedules, even though it's not recommended (and broken for earlier versions of Cassandra).
Fixes #1078