Skip to content

Try both old(9.2) and new cache key generation#12271

Merged
cmcfarlen merged 1 commit intoapache:masterfrom
cmcfarlen:attempt-both-cachekey
Jun 11, 2025
Merged

Try both old(9.2) and new cache key generation#12271
cmcfarlen merged 1 commit intoapache:masterfrom
cmcfarlen:attempt-both-cachekey

Conversation

@cmcfarlen
Copy link
Copy Markdown
Contributor

PR #11519 was a security related PR that removed support for URL param, but also introduced a change that altered the cache key produced from a URL, even if the URL did not have a param component. While this doesn't affect the cache disk layout or cause the cache to reset, it does cause any objects stored by prior versions of ATS to not be found. This has the same effect as resetting the cache when upgrading to ATS10.

This PR mitigates this by introducing a cache key lookup compatibility option. If this option is set, then when ATS encounters a cache miss, it will retry the lookup using the prior versions URL cachekey algorithm. If it then gets a hit, it increments a metric and then proceeds as if the first attempt was found. If it still gets a miss, the transaction will continue on as a miss. In this mode, cache writes will always use the new algorithm and so over time, the need for having this mode enabled goes away.

This route was chosen over reverting the PR because of the security nature of the change and it could be a strategy for future releases to be able to make similar changes to cache and offer a backward compatibility mode for lookups that can be used until the cache wraps.

@cmcfarlen cmcfarlen added this to the 10.2.0 milestone Jun 3, 2025
@cmcfarlen cmcfarlen self-assigned this Jun 3, 2025
@bryancall bryancall requested a review from ezelkow1 June 9, 2025 22:17
Copy link
Copy Markdown
Member

@ezelkow1 ezelkow1 left a comment

Choose a reason for hiding this comment

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

tested here on 10.0.4 and looked good

@cmcfarlen cmcfarlen merged commit 8fbd401 into apache:master Jun 11, 2025
15 checks passed
@github-project-automation github-project-automation Bot moved this to For v10.0.6 in ATS v10.0.x Jun 11, 2025
@github-project-automation github-project-automation Bot moved this to For v10.1.0 in ATS v10.1.x Jun 11, 2025
@cmcfarlen cmcfarlen deleted the attempt-both-cachekey branch June 11, 2025 17:18
cmcfarlen added a commit to cmcfarlen/trafficserver that referenced this pull request Jun 11, 2025
cmcfarlen added a commit that referenced this pull request Jun 11, 2025
@cmcfarlen cmcfarlen removed this from ATS v10.1.x Jun 11, 2025
@cmcfarlen cmcfarlen removed this from the 10.2.0 milestone Jun 11, 2025
@cmcfarlen
Copy link
Copy Markdown
Contributor Author

This was added to milestone 10.1.0 via #12283

@zwoop zwoop added this to the 10.2.0 milestone Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: For v10.0.6

Development

Successfully merging this pull request may close these issues.

3 participants