-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Add "feature-gates" flag and field in config file and add feature gate for StopGRPCServiceOnDefrag #18234
Conversation
@siyuanfoundation: GitHub didn't allow me to request PR reviews from the following users: stackbaek. Note that only etcd-io members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
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.
Nice work @siyuanfoundation - Overall looking good, one main question below about ExperimentalFlags vs FeatureGate handling below.
@@ -907,6 +957,7 @@ func (cfg *Config) Validate() error { | |||
if err := cfg.setupLogging(); err != nil { | |||
return err | |||
} | |||
cfg.ServerFeatureGate.(featuregate.MutableFeatureGate).SetLogger(cfg.logger) |
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.
Why this cannot be done when instantiating config?
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.
because the server logger is not set up before this point. And the ServerFeatureGate could be needed before that, for example there could be a feature gate to set up the logger.
server/etcdmain/help.go
Outdated
@@ -103,6 +103,8 @@ Member: | |||
Read timeout set on each rafthttp connection | |||
--raft-write-timeout '` + rafthttp.DefaultConnWriteTimeout.String() + `' | |||
Write timeout set on each rafthttp connection | |||
--feature-gates '' | |||
A set of key=value pairs that describe feature gates for alpha/experimental features. |
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.
Can we list available feature flags and their defaults?
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.
done.
High level looks good, left some nits. |
1e9ba12
to
e23c9be
Compare
/retest |
Please try not to raise huge PR. |
…e for StopGRPCServiceOnDefrag Signed-off-by: Siyuan Zhang <sizhang@google.com>
part of #18023
This PR includes changes to
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.