-
Notifications
You must be signed in to change notification settings - Fork 82
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 MD.Clone function #177
Conversation
0426d7e
to
288c991
Compare
metadata.go
Outdated
// Preserve nil values. ReverseProxy distinguishes | ||
// between nil and zero-length header values. |
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.
nit: I think we shouldn't copy this comment verbatim from the HTTP implementation. Any mention of a ReverseProxy
is a potential source for confusion, as ttrpc which is typically used for communication between two process on the same physical machine, using a unix domain socket as the transport.
Maybe we should just leave
if vv == nil { // Preserve nil values.
here.
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.
Thanks for the suggestion @klihub . SGTM, updated the PR.
288c991
to
f976f5f
Compare
b80dcd2
to
1a82100
Compare
Signed-off-by: Jin Dong <[email protected]>
1a82100
to
430f734
Compare
Hi @fuweid thanks for approving/merging this PR! I wonder if we could do a ttrpc release recently (ideally also include this dependency upgrade PR #178 but it's optional). This is mainly to fix a concurrent map write panic in otelttrpc (containerd/otelttrpc#2), which is reported in containerd/containerd#11138) thanks |
ping @djdongjin https://github.com/containerd/ttrpc/releases/tag/v1.2.7 this is new tag. |
Great, thanks for the quick release! |
containerd 2.0.2 Welcome to the v2.0.2 release of containerd! The second patch release for containerd 2.0 includes a number of bug fixes and improvements. * Remove confusing warning in cri runtime config migration ([#11256](containerd/containerd#11256)) * Fix runtime platform loading in cri image plugin init ([#11248](containerd/containerd#11248)) * Update runc binary to v1.2.4 ([#11239](containerd/containerd#11239)) Please try out the release binaries and report any issues at https://github.com/containerd/containerd/issues. * Jin Dong * Derek McGowan * Akihiro Suda * Kazuyoshi Kato * Henry Wang * Krisztian Litkey * Phil Estes * Samuel Karp * Sebastiaan van Stijn * Akhil Mohan * Brian Goff * Chongyi Zheng * Maksym Pavlenko * Mike Brown * Pierre Gimalac * Wei Fu <details><summary>23 commits</summary> <p> * Prepare release notes for v2.0.2 ([#11245](containerd/containerd#11245)) * [`cdaf4dfb4`](containerd/containerd@cdaf4df) Prepare release notes for v2.0.2 * Update platforms to latest rc ([#11259](containerd/containerd#11259)) * [`eb125e1dd`](containerd/containerd@eb125e1) Update platforms to latest rc * Remove confusing warning in cri runtime config migration ([#11256](containerd/containerd#11256)) * [`468079c5c`](containerd/containerd@468079c) Remove confusing warning in cri runtime config migration * Fix runtime platform loading in cri image plugin init ([#11248](containerd/containerd#11248)) * [`a2d9d4fd5`](containerd/containerd@a2d9d4f) Fix runtime platform loading in cri image plugin init * make sure console master tty is closed on task exit ([#11246](containerd/containerd#11246)) * [`184ffad01`](containerd/containerd@184ffad) Add integ test to check tty leak * [`17181ed33`](containerd/containerd@17181ed) fix master tty leak due to leaking init container object * Bump up otelttrpc to 0.1.0 ([#11242](containerd/containerd#11242)) * [`8666e7422`](containerd/containerd@8666e74) Bump up otelttrpc to 0.1.0 * ctr: `ctr images import --all-platforms`: fix unpack ([#11236](containerd/containerd#11236)) * [`c4270430d`](containerd/containerd@c427043) ctr: `ctr images import --all-platforms`: fix unpack * Update runc binary to v1.2.4 ([#11239](containerd/containerd#11239)) * [`7373ddd70`](containerd/containerd@7373ddd) update runc binary to v1.2.4 * downgrade go-difflib and go-spew to tagged releases ([#11222](containerd/containerd#11222)) * [`f34147772`](containerd/containerd@f341477) downgrade go-difflib and go-spew to tagged releases * Add a build tag to disable std `plugin` import ([#11213](containerd/containerd#11213)) * [`dca769485`](containerd/containerd@dca7694) chore: add a build tag to disable containerd plugin import * Update golangci to 1.60.3 ([#11187](containerd/containerd#11187)) * [`5942b3fcb`](containerd/containerd@5942b3f) Update golangci to 1.60.3 </p> </details> <details><summary>6 commits</summary> <p> * Add dependabot and upgrade golang and dependency versions ([containerd/otelttrpc#3](containerd/otelttrpc#3)) * [`2d46141`](containerd/otelttrpc@2d46141) upgrade golang, deps, CI versions * [`64922e7`](containerd/otelttrpc@64922e7) Add dependabot CI * Fix concurrent map panic on metadata ([containerd/otelttrpc#2](containerd/otelttrpc#2)) * [`2ba3be1`](containerd/otelttrpc@2ba3be1) Fix concurrent map panic on inject metadata * [`f50a922`](containerd/otelttrpc@f50a922) UT for concurrent inject/extract metadata </p> </details> <details><summary>6 commits</summary> <p> * Move windows matcher logic so all platforms can use ([containerd/platforms#22](containerd/platforms#22)) * [`7c58292`](containerd/platforms@7c58292) Move windows matcher logic so all platforms can use * replace testify with stdlib in tests ([containerd/platforms#21](containerd/platforms#21)) * [`86a86b7`](containerd/platforms@86a86b7) replace testify with stdlib in tests * Replace arm64 minor variant logic with lookup table ([containerd/platforms#18](containerd/platforms#18)) * [`364665a`](containerd/platforms@364665a) Replace arm64 minor variant logic with lookup table </p> </details> <details><summary>5 commits</summary> <p> * Add MD.Clone function ([containerd/ttrpc#177](containerd/ttrpc#177)) * [`430f734`](containerd/ttrpc@430f734) Add MD.Clone * server: fix a Serve() vs. (immediate) Shutdown() race ([containerd/ttrpc#175](containerd/ttrpc#175)) * [`c4d96d5`](containerd/ttrpc@c4d96d5) server: fix Serve() vs. immediate Shutdown() race. * [`ed6c3ba`](containerd/ttrpc@ed6c3ba) server_test: add Serve()/Shutdown() race test. </p> </details> * **github.com/containerd/otelttrpc** ea5083fda723 -> v0.1.0 * **github.com/containerd/platforms** v1.0.0-rc.0 -> v1.0.0-rc.1 * **github.com/containerd/ttrpc** v1.2.6 -> v1.2.7 * **github.com/davecgh/go-spew** d8f796af33cc -> v1.1.1 * **github.com/pmezard/go-difflib** 5d4384ee4fb2 -> v1.0.0 * **github.com/stretchr/testify** v1.9.0 -> v1.10.0 Previous release can be found at [v2.0.1](https://github.com/containerd/containerd/releases/tag/v2.0.1) * `containerd-<VERSION>-<OS>-<ARCH>.tar.gz`: ✅Recommended. Dynamically linked with glibc 2.31 (Ubuntu 20.04). * `containerd-static-<VERSION>-<OS>-<ARCH>.tar.gz`: Statically linked. Expected to be used on non-glibc Linux distributions. Not position-independent. In addition to containerd, typically you will have to install [runc](https://github.com/opencontainers/runc/releases) and [CNI plugins](https://github.com/containernetworking/plugins/releases) from their official sites too. See also the [Getting Started](https://github.com/containerd/containerd/blob/main/docs/getting-started.md) documentation.
MD
is not thread-safe so sometimes we need to make a copy before modification, e.g. in containerd,https://github.com/containerd/containerd/blob/c3efa0cb339ffc2d6c9f03d7fb4e5e6577d0194b/pkg/namespaces/ttrpc.go#L30-L36
This PR adds a
MD.Clone
function, similar to golang'shttp.Header.Clone
(the code is copied there). Also add a benchmark that shows golang's implementation is better than a simple copy approach (both memory&latency):