pkg/fqdn: Fix missing delete for forward map #19925
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/test |
schlosna
reviewed
May 24, 2022
nebril
approved these changes
May 24, 2022
tommyp1ckles
approved these changes
May 24, 2022
|
/mlh new-flake Cilium-PR-Runtime-net-next 👍 created #19950 Edit: CI passed except for known flake above. Pushing to address #19925 (comment). |
2eaa819 to
e6546ac
Compare
This commit fixes a bug where the keys of the forward map inside the DNS cache were never removed, causing the map to grow forever. By contrast, the reverse map keys were being deleted. For both the forward and reverse maps (which are both maps whose values are another map), the inner map keys were being deleted. In other words, the delete on the outer map key was missing for the forward map. In addition to fixing the bug, this commit expands the unit test coverage to assert after any deletes (entries expiring or GC) that the forward and reverse maps contain what we expect. Particularly, in an environment where there are many unique DNS lookups (unique FQDNs) being done, this forward map could grow quite large over time, especially for a long-lived workload (endpoint). This fixes this memory-leak-like bug. Fixes: cf387ce ("fqdn: Introduce TTL-aware cache for DNS retention") Fixes: f6ce522 ("FQDN: Added garbage collector functions.") Signed-off-by: Chris Tarazi <chris@isovalent.com>
e6546ac to
04317b0
Compare
joestringer
approved these changes
May 24, 2022
|
/test |
jrajahalme
approved these changes
Jun 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes a bug where the keys of the forward map inside the DNS
cache were never removed, causing the map to grow forever. By contrast,
the reverse map keys were being deleted.
For both the forward and reverse maps (which are both maps whose values
are another map), the inner map keys were being deleted.
In other words, the delete on the outer map key was missing for the
forward map.
In addition to fixing the bug, this commit expands the unit test
coverage to assert after any deletes (entries expiring or GC) that the
forward and reverse maps contain what we expect.
Particularly, in an environment where there are many unique DNS lookups
(unique FQDNs) being done, this forward map could grow quite large over
time, especially for a long-lived workload (endpoint). This fixes this
memory-leak-like bug.
Fixes: cf387ce ("fqdn: Introduce TTL-aware cache for DNS retention")
Fixes: f6ce522 ("FQDN: Added garbage collector functions.")
Signed-off-by: Chris Tarazi chris@isovalent.com