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

Bump dependencies #16210

Merged
merged 4 commits into from
Jul 10, 2023
Merged

Bump dependencies #16210

merged 4 commits into from
Jul 10, 2023

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Jul 10, 2023

Please see commit messages, each of them bumps one dependency.

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

Signed-off-by: Benjamin Wang <wachao@vmware.com>
Signed-off-by: Benjamin Wang <wachao@vmware.com>
Signed-off-by: Benjamin Wang <wachao@vmware.com>
Signed-off-by: Benjamin Wang <wachao@vmware.com>
@ahrtr
Copy link
Member Author

ahrtr commented Jul 10, 2023

cc @ptabor @spzala @serathius @wenjiaswe

@chaochn47
Copy link
Member

As part of the net stdlib update, from what I read from golang/go#58804 (comment), we should switch back http server to use default scheduler, is that understanding correct?

etcd/server/embed/serve.go

Lines 269 to 276 in 1f07d8f

func configureHttpServer(srv *http.Server, cfg config.ServerConfig) error {
// todo (ahrtr): should we support configuring other parameters in the future as well?
return http2.ConfigureServer(srv, &http2.Server{
MaxConcurrentStreams: cfg.MaxConcurrentStreams,
// Override to avoid using priority scheduler which is affected by https://github.com/golang/go/issues/58804.
NewWriteScheduler: http2.NewRandomWriteScheduler,
})
}

@ahrtr
Copy link
Member Author

ahrtr commented Jul 10, 2023

@chaochn47 It's an unrelated comment, please raise the comment in #15402.

Per my understanding, with the merge of 120fc90, the production code change in #15431 isn't necessary, but it will NOT do any harm and the test case still makes sense.

BTW, the golang std lid commit 260a9b0a0 will only be included in golang 1.21+.

@chaochn47
Copy link
Member

It's an unrelated comment, please raise the comment in #15402.

Thanks @ahrtr! I will move the discussion to the right place.

This PR looks good to me.

Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM

@ahrtr
Copy link
Member Author

ahrtr commented Jul 10, 2023

thx @chaochn47 and @jmhbnz for the quick review.

@ahrtr ahrtr merged commit c853b9c into etcd-io:main Jul 10, 2023
@ahrtr ahrtr deleted the dependency_20230710 branch June 12, 2024 11:40
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.

3 participants