Skip to content
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

[3.5] add experimental-snapshot-catchup-entries flag #17808

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

siyuanfoundation
Copy link
Contributor

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

#17752

This is import prerequisite to verify that in a mixed version cluster, the follower receives a snapshot that has the correct schema for that version.

Did not add the flag to help file so that it is a hidden flag that only is used for testing.

@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@siyuanfoundation
Copy link
Contributor Author

@serathius @ahrtr

@siyuanfoundation
Copy link
Contributor Author

/retest

server/etcdmain/config.go Outdated Show resolved Hide resolved
@@ -302,6 +302,7 @@ func newConfig() *config {
fs.BoolVar(&cfg.ec.ExperimentalMemoryMlock, "experimental-memory-mlock", cfg.ec.ExperimentalMemoryMlock, "Enable to enforce etcd pages (in particular bbolt) to stay in RAM.")
fs.BoolVar(&cfg.ec.ExperimentalTxnModeWriteWithSharedBuffer, "experimental-txn-mode-write-with-shared-buffer", true, "Enable the write transaction to use a shared buffer in its readonly check operations.")
fs.UintVar(&cfg.ec.ExperimentalBootstrapDefragThresholdMegabytes, "experimental-bootstrap-defrag-threshold-megabytes", 0, "Enable the defrag during etcd server bootstrap on condition that it will free at least the provided threshold of disk space. Needs to be set to non-zero value to take effect.")
fs.Uint64Var(&cfg.ec.SnapshotCatchUpEntries, "experimental-snapshot-catchup-entries", cfg.ec.SnapshotCatchUpEntries, "(WARNING: Use this flag with caution!) Number of entries for a slow follower to catch up after compacting the raft storage entries.")
Copy link
Member

Choose a reason for hiding this comment

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

I think we should add similar "WARNING: Use this flag with caution!" for the main branch as well.

Copy link
Member

Choose a reason for hiding this comment

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

Can you provide more context why you think that? The original PR added the warning because we are backporting a feature just for testing, and we didn't want to make it visible to users.

Copy link
Member

Choose a reason for hiding this comment

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

The original PR added the warning because we are backporting a feature just for testing

I don't think it's a good idea to add a user-facing flag but only supposed to be used by test. #17808 (comment)

Copy link
Member

@serathius serathius Apr 18, 2024

Choose a reason for hiding this comment

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

This is only a test only backport, but the original flag was introduced in #15033 with broader set of motivations. I'm mentioning it to ensure we discuss it before we add the warning on the main branch.

@ahrtr
Copy link
Member

ahrtr commented Apr 18, 2024

Please signoff the commit

@serathius serathius merged commit 91e9dd3 into etcd-io:release-3.5 Apr 19, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants