-
Notifications
You must be signed in to change notification settings - Fork 2.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
pkg/cgroups: Cache pod metadata on datapath events #32615
pkg/cgroups: Cache pod metadata on datapath events #32615
Conversation
5fdb0ae
to
d6b947f
Compare
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
d6b947f
to
c5c4ec9
Compare
c5c4ec9
to
da60744
Compare
/ci-runtime |
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.
Some minor further comments on the use of WaitGroups for synchronizing the unit tests - The usage doesn't seem to match canonical Go usage, and doesn't match up with the API comments in the library package. Fixing that up by passing it as a parameter or switching to channels should be a simple change though AFAICT.
The test case fails without the setup when run by itself. Fixes: 92ea5a1 ("pkg/cgroups: Replace gocheck with built-in go test") Signed-off-by: Aditi Ghag <aditi@cilium.io>
Signed-off-by: Aditi Ghag <aditi@cilium.io>
da60744
to
b99accf
Compare
/ci-runtime |
b99accf
to
be4655d
Compare
/ci-runtime |
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.
This solution ended up even tidier than what I had in mind. Nice one 👍
Cgroup manager events were previously synchronized via a channel. However, Hubble calls GetPodMetadataForContainer on datapath socket tracing events that can be generated much more frequently than control plane events. Add a cache for the datapath read events in order to reduce the overhead associated with channel events processing. This would mean that the datapath events may get data via cache while any pod delete events are in progress. Since only the control plane events are required to be synchronized, it's acceptable that datapath tracing events may see cached data for a short duration. Benchmark results: Before: BenchmarkGetPodMetadataForContainer-14 848550 1383 ns/op After: BenchmarkGetPodMetadataForContainer-14 2885898 408.7 ns/op Reported-by: Joe Stringer <joe@cilium.io> Signed-off-by: Aditi Ghag <aditi@cilium.io>
be4655d
to
c730e61
Compare
/ci-runtime |
/test |
See commits description.