Skip to content

Comments

Add docs on delegated Raft snapshots#16876

Merged
rmloveland merged 1 commit intomasterfrom
20230502-DOC-6815-delegated-raft-snapshots
May 15, 2023
Merged

Add docs on delegated Raft snapshots#16876
rmloveland merged 1 commit intomasterfrom
20230502-DOC-6815-delegated-raft-snapshots

Conversation

@rmloveland
Copy link
Contributor

@github-actions
Copy link

github-actions bot commented May 2, 2023

Files changed:

@rmloveland
Copy link
Contributor Author

@andrewbaptist it appears the cluster settings in the code in cockroachdb/cockroach#83991 were different than the ones in the PR comment, so I went with the former

Open to not including the description of the settings so explicitly, since it's not clear we necessarily want folks changing them? In any case they will appear in the autogenerated cluster settings docs for the very curious

This is mostly based on a mix of the PR commit message and the press release Gdoc. Please feel free to suggest any corrections/improvements as you see fit of course!

Copy link

@andrewbaptist andrewbaptist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this, a few small changes.


Delegated snapshots are enabled by default, and managed automatically by the cluster with no need for user involvement. If you want to control how delegated snapshots work, the following cluster settings are available:

- [`kv.snapshot_delegation.num_follower`](../cluster-settings.html#setting-kv-snapshot-delegation-num-follower): The number of delegates to try when sending snapshots, before falling back to sending from the leaseholder. If set to `0`, snapshot delegation is disabled.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if you want to mention this only applies if the entire cluster is on v23.1

The settings were renamed I checked the code and they are currently. (Sorry for the confusion)

kv.snapshot_delegation.max_delegation_attempts (replaces num_followers)
kv.snapshot_delegations.queue_limit (replaces num_requests)

See https://github.com/cockroachdb/cockroach/blob/master/pkg/kv/kvserver/replica_command.go#L2978 for details (including the help text).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updated info. I took another look and it appears these settings are not marked as public in the code so they are not appearing in the v23.1 cluster settings docs (which are generated automatically from the cockroach repo, but only for "public" settings).

Was that intentional? If so I would recommend we remove the cluster setting info from this PR for consistency with how we document other cluster settings.

If this was not intentional and these settings should be "public", I can add some info here from your comment. For now I have removed mention of the cluster settings

Advanced users can always see all the non-public cluster settings and their help text using SHOW ALL CLUSTER SETTINGS if they want to.

PTAL and let me know what you think

Copy link

@andrewbaptist andrewbaptist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this, a few small changes.

@rmloveland rmloveland force-pushed the 20230502-DOC-6815-delegated-raft-snapshots branch from e48f56f to 919fa69 Compare May 10, 2023 15:49
Copy link

@andrewbaptist andrewbaptist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@rmloveland rmloveland requested a review from ianjevans May 15, 2023 14:56
Copy link
Contributor

@ianjevans ianjevans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but see my comment about possibly linking to the setting that enables/disables delegated snapshots.

@rmloveland rmloveland force-pushed the 20230502-DOC-6815-delegated-raft-snapshots branch from 919fa69 to 0df7ee0 Compare May 15, 2023 16:55
@netlify
Copy link

netlify bot commented May 15, 2023

Netlify Preview

Name Link
🔨 Latest commit 0df7ee0
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/6462640eae492f00089e32cf
😎 Deploy Preview https://deploy-preview-16876--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@rmloveland rmloveland merged commit abb966a into master May 15, 2023
@rmloveland rmloveland deleted the 20230502-DOC-6815-delegated-raft-snapshots branch May 15, 2023 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants