-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
doc: update Raft information in 6.0 #18689
Conversation
Docs Preview 📖Docs Preview for this pull request is available here Changed Files: Note: This preview will be available for 30 days and will be automatically deleted after that period. You can manually trigger a new build by committing changes. |
docs/architecture/raft.rst
Outdated
|
||
.. _verify-raft-procedure: | ||
|
||
Verifying that the Raft upgrade procedure finished successfully | ||
======================================================================== | ||
|
||
.. scylladb_include_flag:: note-enabling-consistent-topology-changes.rst | ||
You must perform the following procedure only once - when the Raft-based | ||
schema changes feature has been enabled in your cluster for the first time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You must also perform it after going through the manual Raft recovery procedure (https://opensource.docs.scylladb.com/master/troubleshooting/handling-node-failures.html#manual-recovery-procedure)
every time one does the manual Raft recovery procedure (hopefully never, because it's for disaster scenarios -- loss of majority), one has to do this validation.
Perhaps we can preamble the statement with "Normally" or something. "Normally, you must perform ..."
docs/architecture/raft.rst
Outdated
|
||
.. _verify-raft-procedure: | ||
|
||
Verifying that the Raft upgrade procedure finished successfully | ||
======================================================================== | ||
|
||
.. scylladb_include_flag:: note-enabling-consistent-topology-changes.rst | ||
You must perform the following procedure only once - when the Raft-based | ||
schema changes feature has been enabled in your cluster for the first time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to explain when this happens? What is the trigger for the schema changes feature to get enabled? If we don't explain it -- how will the user know when to actually perform this procedure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've made a good point, obviously. This section doesn't make it clear. In previous releases, we added the information WHEN the procedure should be performed to the upgrade guides - with this section serving as a reference.
I've added the above sentence for the sake of clarification, but it might make the message more vague.
I could explain the details, but it would involve adding more release-specific information in additional include files.
I'd lean to taking the approach we had in previous releases - adding release-specific information in the upgrade guide, and removing the above sentence altogether.
@kbr-scylla Are you OK with that? We'll simply keep the version we had in previous releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it is possible to also enable Raft in a, let's say, 5.2 cluster, by setting consistent_cluster_management: true
(when it was previously false
) and doing a rolling restart --- disconnected from the version upgrade procedure.
But this is 6.0 documentation, so I think we should assume that whoever is reading this article, is reading it in the context of running a 6.0 cluster or upgrading their cluster to 6.0.
So with that assumption, we could just say something like:
"If you disabled Raft in 5.4 (by setting consistent_cluster_management: false
), the Raft-based schema changes feature will automatically get enabled once you finish rolling-upgrade to 6.0, and then you must perform the verification procedure."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this is 6.0 documentation, so I think we should assume that whoever is reading this article, is reading it in the context of running a 6.0 cluster or upgrading their cluster to 6.0.
Yes, we shouldn't consider 5.2 or any other version other than 6.0.
So with that assumption, we could just say something like: "If you disabled Raft in 5.4 (by setting
consistent_cluster_management: false
), the Raft-based schema changes feature will automatically get enabled once you finish rolling-upgrade to 6.0, and then you must perform the verification procedure."
I totally agree with the message. I just think that it should be added to the upgrade guide (I'm working on it now). Upgrade-related details on the Raft page are not helpful, they must be part of the upgrade procedure.
I'll just restore this section to what it was in previous releases, i.e., I'll remove the unfortunate sentence I've added above.
If you insist we should have something, it could be: "You may need to perform the following procedure on upgrade if you explicitly disabled the Raft-based schema changes feature in the previous ScyllaDB version. Please consult the upgrade guide." - without version numbers or links (which will cause issues on merge with enterprise).
Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kbr-scylla I've applied the update. Even if not perfect, it still adds some clarity we didn't have in previous versions. I don't wan't to add any links here. Please have a look.
This commit updates the documentation about Raft in version 6.0. - "Introduction": The outdated information about consistent topology updates not being supported is removed and replaced with the correct information. - "Enabling Raft": The relevant information is moved to other sections. The irrelevant information is removed. The section no longer exists. - "Verifying that the Raft upgrade procedure finished successfully" - moved under Schema (in the same document). I additionally removed the include saying that after you verify that schema on Raft is enabled, you MUST enable topology changes on Raft (it is not mandatory; also, it should be part of the upgrade guide, not the Raft document). - Unnecessary or incorrect references to versions are removed. Refs scylladb#18580
d3b2315
to
ecb1dbd
Compare
This commit updates the documentation about Raft in version 6.0.
Refs #18580