[
https://issues.apache.org/jira/browse/CASSANDRA-21595?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Erick Ramirez updated CASSANDRA-21595:
--------------------------------------
Reviewers: Erick Ramirez
Status: Review In Progress (was: Patch Available)
[~aviau] Thanks for turning around all the review feedback so quickly. I've
done a full pass on the three PRs and there are just a couple of things left.
They apply to all three branches since the patch is identical.
*1. The "usually aborts" case in the docs*
In the {{unset (default)}} row of the new table, the docs now say:
{quote}If ranges from a previous bootstrap are found, the bootstrap usually
aborts with an error, so that no decision about the existing data is made on
your behalf. If none are found, the bootstrap proceeds normally.
{quote}
"Usually" is accurate, but it leaves the reader guessing what happens the rest
of the time, which is the gap this ticket set out to close. In
{{{}RangeStreamer.fetchAsync(){}}}, the abort only happens when
{{{}remaining.size() < available.full.size() + available.trans.size(){}}}. When
that condition isn't met, the node quietly skips the ranges that are already
available, the same as {{{}false{}}}, except that it doesn't log the "Some
ranges ... are already available" line that {{false}} does. I think that's the
paragraph you and [~gandherva] agreed on, but it hasn't made it into the PRs
yet.
Consider adding it after the table, something like:
{quote}In some situations, a bootstrap with the property unset doesn't abort.
Instead, it continues and reuses the ranges that a previous attempt already
streamed, without logging that it skipped them. To avoid this, we recommend
setting the property to {{true}} after removing the files from the previous
attempt.
{quote}
*2. Showing the {{-D}} form in the error message*
The new message reads:
{quote}... and cassandra.reset_bootstrap_progress is unset; aborting bootstrap.
Set it to 'true' to re-stream all ranges (delete the data files from the
previous attempt first), or to 'false' to stream only the missing ranges ...
{quote}
It's much clearer than the original, and naming the property once was a good
call. The only thing I'd add is how to set it. An operator reading this in
{{system.log}} is mid-incident, and "Set it to 'true'" doesn't say that it's a
JVM property that needs a restart. [~mck]'s earlier suggestion covered this
with {{{}-D%1$s=true{}}}.
Consider keeping your structure and closing with the flag, which only names the
property one more time:
{code:java}
String msg = String.format("Discovered existing bootstrap data (ranges already
marked as available by a previous " +
"bootstrap attempt) and %1$s is unset; aborting
bootstrap. To start a new bootstrap and " +
"re-stream all ranges, delete the data files from
the previous attempt and set it to 'true'. " +
"To stream only the missing ranges, set it to
'false' (potentially dangerous, as the previous " +
"data may be incomplete). Set it on restart with
-D%1$s=<true|false>. See CASSANDRA-17679. " +
"Found: %2$s. Fully available: %3$s. Transiently
available: %4$s",
RESET_BOOTSTRAP_PROGRESS.getKey(), skipped,
available.full, available.trans);
{code}
{{BootstrapTest}} matches on "Discovered existing bootstrap data", so the test
is unaffected either way.
Once these are in and the screenshots from the test plan are attached, I'll
stage the updates locally as the final verification step before merging the
PRs. Cheers! 🍻
> docs: distinguish between reset_bootstrap_progress (true, false, unset)
> -----------------------------------------------------------------------
>
> Key: CASSANDRA-21595
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21595
> Project: Apache Cassandra
> Issue Type: Improvement
> Components: Consistency/Bootstrap and Decommission, Documentation
> Reporter: Alexandre Viau
> Assignee: Alexandre Viau
> Priority: Normal
> Labels: patch-available
> Fix For: 5.0.x, 6.0.x, 7.0
>
> Time Spent: 2h 50m
> Remaining Estimate: 0h
>
> The only documenation available on `reset_bootstrap_progress` is the
> following:
> > In order to cleanup bootstrap state and start fresh, you may set the JVM
> > startup flag -Dcassandra.reset_bootstrap_progress=true.
> This gives the impression that if I don't want to start fresh, I should not
> do anything. However, this is false. If I want my node to resume bootstraping
> with existing data, I need to set it to `false`, which is different than the
> default of `unset`.
> I think this is impossible to infer from existing documentation. Only reading
> source code helped me understand.
>
> I opened a Merge Request on github here:
> https://github.com/apache/cassandra/pull/5047
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
