Fix OOM and Handler timeout issue by only returning one item in ListAllMetrics by default
#623
Conversation
This file contains 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
CatherineF-dev
force-pushed
the
remove-list
branch
from
32a26a4 to
6d08ee5
Compare
CatherineF-dev
changed the title
CatherineF-dev
force-pushed
the
remove-list
branch
from
6d08ee5 to
5a3bd71
Compare
|
Do we know the root cause for "http2: stream closed" and could you elaborate how returning empty ListAllMetrics will solve this issue? |
|
ListAllMetrics is called during api discovery and isn't needed for HPA. In this HPA case, if #target pods is 5, it will call this function 5 times at the same time. It's not scalable for a large cluster. In my small cluster, during 60s, ListAllMetrics is called 11 times and each time returns 864502*2 bytes=1.72 megabytes
It's a timeout issue, so it will be fixed after returning empty vaules. |
|
/retest I just added more CI pipelines. |
CatherineF-dev
force-pushed
the
remove-list
branch
3 times, most recently
from
55df82b to
a983c73
Compare
CatherineF-dev
changed the title
courageJ
reviewed
Jan 11, 2024
courageJ
approved these changes
Jan 14, 2024
…item for ListCustomMetrics
CatherineF-dev
force-pushed
the
remove-list
branch
from
ee5a839 to
673d295
Compare
|
Hi, google sent me here :) I'm happy to see this issue has a fix, I was wondering how I could implement it in our GKE. I tried to edit manually the file Thanks a lot for you attention :) |
|
Hi @slash4, could you check the version by I didn't see this error in my cluster, will use custom-metrics-stackdriver-adapter/deploy/production/adapter_new_resource_model.yaml to reproduce. |
|
Sure : here's the output of the command 👍
I'm so grateful you responded so quickly. Thanks :) |
CatherineF-dev
changed the title
Handler timeout issue by only returning one item in ListAllMetrics by default
|
btw, I found this one https://stackoverflow.com/questions/67073909/error-scaling-up-in-hpa-in-gke-apiserver-was-unable-to-write-a-json-response-h, could you try this? |
|
|
Regarding this : https://stackoverflow.com/questions/67073909/error-scaling-up-in-hpa-in-gke-apiserver-was-unable-to-write-a-json-response-h I'm already in 
Could it be the |
|
Ok, I understand that spam logs are annoying. If I need more information, will let you know. |
|
Thanks a lot ! Sure, I'm here if you need me :) |
|
Hi @slash4, does your cluster still have spam logs? Just want to see whether spam logs are gone after x hours. |
|
@CatherineF-dev @slash4 does the spam logs are gone now? if yes please let me know what was the root cause, since we are also seeing the same errors |
|
Could you provide detailed steps to reproduce? @hariapollo Are you using the latest custom-metrics-stackdriver-adapter? |
|
Hey @CatherineF-dev, We were on |
|
After upgrading it to |
|
@hariapollo could you open a new issue? Trace logs are not related to this issue. They are ignorable. |
|
Sure, you mean for cert auth issue right? |
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.
Adapted from #311
Also,
Instead of returning empty, this PR returns 1 metric resource item. Because returning empty response is not fine for generic clients that list resources with api discovery, like the namespace garbage collector and GitOps serivces like Config Sync and ArgoCD. An APIService that returns an empty list is invalid and causes an error in client-go.
added a feature-gate
list-full-custom-metricswith default value = false.metricsCache is the same as before previous.
Fixes: #582, #545, #510, #458
Tested:
The memory drop will be more significant in a large cluster.
"http2: stream closed" is from calling
/apis/custom.metrics.k8s.io/v1beta2. Since it only returns 1 item, it's hard to have timeout error.ListMetrics is not used in HPA, so it's safe to change it.
Cons:
API discovery around /apis/custom.metrics.k8s.io/v1beta2 returns an incomplete resource list, instead of listing all available metrics. This is fine since customers can find full metric names from GCP monitoring dashboards.The feature-gate
list-full-custom-metricsreturns all custom-metrics when it's true.