fix PILOT_ENABLE_RDS_CACHE flag not working #40719
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
When building RDS, cache get is always conducted not respecting the PILOT_ENABLE_RDS_CACHE flag. This CL fixes that. Cache add already respects the flag: https://github.com/airbnb/istio/blob/1.14.3-airbnb1/pilot/pkg/networking/core/v1alpha3/httproute.go#L212 Change-Id: Ic0634647239a6def8ced4ce611f5a986f130c95a Reviewed-on: https://gerrit.musta.ch/c/public/istio/+/3611 Reviewed-by: Douglas Jordan <douglas.jordan@airbnb.com> Reviewed-by: Jungho Ahn <jungho.ahn@airbnb.com> Reviewed-by: Weibo He <weibo.he@airbnb.com>
|
🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test? Courtesy of your friendly test nag. |
hzxuzhonghu
reviewed
Aug 31, 2022
| resource, exist := xdsCache.Get(routeCache) | ||
| if exist && !features.EnableUnsafeAssertions { | ||
| return nil, resource, routeCache | ||
| if features.EnableRDSCaching { |
There was a problem hiding this comment.
why is that? for cds we do respect it for get as well
https://github.com/istio/istio/blob/1.14.3/pilot/pkg/networking/core/v1alpha3/cluster_builder.go#L1271
|
So what is this fixing? |
|
seems performance optimization for when its disabled? LGTM |
|
yes, disabling it helps with propagation delay. I posted my testing here: #40744 |
airbnb-gerrit
pushed a commit
to airbnb/istio
that referenced
this pull request
Sep 1, 2022
This CL patches commit b0db731 from upstream istio into air-release-1.13.3 to fix the PILOT_ENABLE_RDS_CACHE flag. Original PR can be found here: istio#40719. The description from that PR is reproduced below. When building RDS, cache get is always conducted not respecting the PILOT_ENABLE_RDS_CACHE flag. This PR fixes that. Cache add already respects the flag: https://github.com/istio/istio/blob/1.14.3/pilot/pkg/networking/core/v1alpha3/httproute.go#L212 Change-Id: I5080a152caef9ff55cf23e958e75de7531d56d47 Reviewed-on: https://gerrit.musta.ch/c/public/istio/+/3625 Reviewed-by: Ryan Smick <ryan.smick@airbnb.com>
airbnb-gerrit
pushed a commit
to airbnb/istio
that referenced
this pull request
Sep 13, 2022
Apply the following list of patches to istio 1.14.4: * sidecar: filter service ports to VS ports (istio#39067) * istio: register init push context metric (istio#40049) * istio: add metric for debouncing (istio#40523) * istio: fix PILOT_ENABLE_RDS_CACHE flag not working (istio#40719) * istio: support inline multi-values header in authz header match (https://gerrit.musta.ch/c/public/istio/+/3622, not yet merged upstream) Change-Id: I69b861d884608dabad44db1fee03f66eb4b25ab2 Reviewed-on: https://gerrit.musta.ch/c/public/istio/+/3695 Reviewed-by: Ying Zhu <ying.zhu@airbnb.com> Reviewed-by: Weibo He <weibo.he@airbnb.com>
airbnb-gerrit
pushed a commit
to airbnb/istio
that referenced
this pull request
Oct 12, 2022
Apply the following list of patches to istio 1.14.5: * sidecar: filter service ports to VS ports (istio#39067) * istio: register init push context metric (istio#40049) * istio: add metric for debouncing (istio#40523) * istio: fix PILOT_ENABLE_RDS_CACHE flag not working (istio#40719) * istio: support inline multi-values header in authz header match (https://gerrit.musta.ch/c/public/istio/+/3622, not yet merged upstream) * istio: improve deep copy for ServiceAttribute (istio#40966) * avoid unnecessary copy virtual services for sidecar scope calculation (istio#41101) Change-Id: Ia4c9bfd619a0eb38c1a829bff2efbd21fd3b9cb2 Reviewed-on: https://gerrit.musta.ch/c/public/istio/+/3883 Reviewed-by: Ying Zhu <ying.zhu@airbnb.com> Reviewed-by: Weibo He <weibo.he@airbnb.com>
airbnb-gerrit
pushed a commit
to airbnb/istio
that referenced
this pull request
Nov 10, 2022
Apply the following list of upstream commits to istio 1.15.3: * istio: add metric for debouncing (istio#40523) * istio: fix PILOT_ENABLE_RDS_CACHE flag not working (istio#40719) * istio: improve deep copy for ServiceAttribute (istio#40966) * avoid unnecessary copy virtual services for sidecar scope calculation (istio#41101) Change-Id: I2ee1d77d096a329dc8f590151223b37193dd4f1b Reviewed-on: https://gerrit.musta.ch/c/public/istio/+/3990 Reviewed-by: Ying Zhu <ying.zhu@airbnb.com> Reviewed-by: Ryan Smick <ryan.smick@airbnb.com>
|
/cherrypick release-1.15 |
|
@S-Chan: new pull request created: #42669 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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.
Please provide a description of this PR:
When building RDS, cache get is always conducted not respecting the
PILOT_ENABLE_RDS_CACHE flag. This PR fixes that.
Cache add already respects the flag:
https://github.com/istio/istio/blob/1.14.3/pilot/pkg/networking/core/v1alpha3/httproute.go#L212