Skip to content

Do not turn off cache for internal requests #8266

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.

Sign up for GitHub

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

Jump to bottom
Merged
vmamidi merged 5 commits into from
Jan 25, 2022
Merged

Do not turn off cache for internal requests #8266

vmamidi merged 5 commits into from
Jan 25, 2022

Conversation

@vmamidi
Copy link
Contributor

@vmamidi vmamidi commented Aug 19, 2021 •
edited
Loading

As of now, auth proxy disables cache for all the internal requests. Auth proxy disables the cache for any plugin that generates internal requests and goes through the same remap rule. For example, background fetch plugin which generates a internal request doesn't cache internal requests.

If there is a requirement to disable caching, it can be achieved by using the conf_remap plugin.

Update: retained the existing behavior and added a variable to enable caching

@vmamidi vmamidi added this to the 9.1.0 milestone Aug 19, 2021
@vmamidi vmamidi self-assigned this Aug 19, 2021
@vmamidi vmamidi requested a review from bryancall as a code owner August 19, 2021 19:16
zwoop
zwoop previously requested changes Aug 19, 2021
View reviewed changes
Copy link
Contributor

@zwoop zwoop left a comment

Choose a reason for hiding this comment

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

How does this work to make sure it doesn’t serve the auth request via a cached response?

@vmamidi
Copy link
Contributor Author

vmamidi commented Aug 19, 2021

How does this work to make sure it doesn’t serve the auth request via a cached response?

The plugin doesn't make sure that auth requests are not served from the cache. Disabling cache in auth proxy for existing remap( existing behavior) also doesn't guarantee that as auth requests may go to a different remap rule.

@randall randall modified the milestones: 9.1.0, 10.0.0 Aug 23, 2021
@bryancall bryancall marked this pull request as draft August 23, 2021 23:11
@bryancall
Copy link
Contributor

bryancall commented Oct 14, 2021

[approve ci]

@vmamidi
Copy link
Contributor Author

vmamidi commented Dec 13, 2021

How does this work to make sure it doesn’t serve the auth request via a cached response?

The plugin doesn't make sure that auth requests are not served from the cache. Disabling cache in auth proxy for existing remap( existing behavior) also doesn't guarantee that as auth requests may go to a different remap rule.

Added configuration to enable/disable caching.

@vmamidi vmamidi marked this pull request as ready for review December 13, 2021 19:36
@vmamidi vmamidi requested a review from zwoop December 13, 2021 19:37
@vmamidi vmamidi dismissed zwoop’s stale review December 13, 2021 20:01

addressed in the latest commit

@vmamidi
Copy link
Contributor Author

vmamidi commented Dec 13, 2021

[approve ci]

1 similar comment
@vmamidi
Copy link
Contributor Author

vmamidi commented Dec 16, 2021

[approve ci]

bryancall
bryancall requested changes Jan 11, 2022
View reviewed changes
Copy link
Contributor

@bryancall bryancall left a comment

Choose a reason for hiding this comment

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

please update the docs for the PR

@vmamidi vmamidi requested a review from bryancall January 11, 2022 18:57
@vmamidi
Copy link
Contributor Author

vmamidi commented Jan 11, 2022

please update the docs for the PR

updated.

@vmamidi
Copy link
Contributor Author

vmamidi commented Jan 11, 2022

[approve ci]

@vmamidi vmamidi merged commit 33cc799 into apache:master Jan 25, 2022
zwoop pushed a commit that referenced this pull request Jan 25, 2022
@vmamidi @zwoop
Do not turn off cache for internal requests (#8266)
5998807 
* Do not turn off cache for internal requests

* Allow configuration to enable/disable caching internal requests

* do not enable cache

* documentation updated

(cherry picked from commit 33cc799)
@zwoop
Copy link
Contributor

zwoop commented Jan 25, 2022

Cherry-picked to v9.2.x

@zwoop zwoop removed this from the 10.0.0 milestone Jan 25, 2022
@zwoop zwoop added this to the 9.2.0 milestone Jan 25, 2022
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Mar 17, 2022
@zwoop
Merge remote-tracking branch 'asf/9.2.x' into 9.2.x
8ec090e 
* asf/9.2.x:
  Updated ChangeLog
  Add SSLSessionDup for older OpenSSL and BoringSSL (apache#8578)
  use shared pointer to help with high memory utilization (apache#8498)
  Commenting TSHttpTxnCacheLookupStatusGet need_to_revalidate (apache#8621)
  check size of session, and free sessions the ATS way (apache#8330)
  free sessions when timeout (apache#8356)
  Fix 32bit build failure on Odroid Xu-4 (apache#8626)
  TSHttpTxnCacheLookupStatusGet: call need_to_revalidate (apache#8617)
  SNIConfig (tunnel_route): Change the way we extract matched subgroups from the server name. (apache#8589)
  fix for collapsed forwarding ink_abort for CacheHitFresh fail (apache#8613)
  Do not turn off cache for internal requests (apache#8266)
  Rate Limit Plugin: Re-enable VConnection when SNI is empty (apache#8625)
  Removes hard dependency on having perl installed (apache#8611)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bryancall bryancall bryancall approved these changes

@zwoop zwoop Awaiting requested review from zwoop

Assignees

@vmamidi vmamidi

Labels

New Feature Plugins

Projects

None yet

Milestone

9.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@vmamidi @bryancall @zwoop @randall