Skip to content
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

Non-Active Revisions with ScaleToZeroPodRetention and min-non-active-revisions > 1 is not scaled down #13812

Open
AswinT22 opened this issue Mar 23, 2023 · 12 comments · May be fixed by #15161
Open

Non-Active Revisions with ScaleToZeroPodRetention and min-non-active-revisions > 1 is not scaled down #13812

AswinT22 opened this issue Mar 23, 2023 · 12 comments · May be fixed by #15161
Assignees
@eddy-oum
Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Issues which should be fixed (post-triage)
Milestone
v1.15.0

Comments

@AswinT22
Copy link

What version of Knative?

1.9.2

Expected Behavior

Non active must be scaled down with autoscaling.knative.dev/scale-to-zero-pod-retention-period set

Actual Behavior

Non active revisions are not scaled down

Steps to Reproduce the Problem

Deploy a Knative service with scale-to-zero-pod-retention-period (make sure this value is higher so you will be able to see the issue)

apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  name: hello-example
  namespace: default
spec:
  template:
    metadata:
      annotations:
        autoscaling.knative.dev/initial-scale: "1"
        autoscaling.knative.dev/min-scale: "0"
        autoscaling.knative.dev/scale-to-zero-pod-retention-period: "1h"
    spec:
      containerConcurrency: 0
      containers:
      - env:
        - name: TARGET
          value: World
        image: gcr.io/knative-samples/helloworld-go
        name: user-container

Edit the config-map config-gc and set below values

max-non-active-revisions: "2"
min-non-active-revisions: "1"
retain-since-create-time: disabled
retain-since-last-active-time: disabled
  • Deploy The knative service
  • Then alter the service to deploy a newer revisions
  • You will see the old revision not scaling down
@AswinT22 AswinT22 added the kind/bug Categorizes issue or PR as related to a bug. label Mar 23, 2023
@dprotaso
Copy link
Member

autoscaling.knative.dev/scale-to-zero-pod-retention-period: "1h"

I lowered this from an hour (1h) to 10 miuntes (10m) and the pod scaled down after 10 minutes. Are you seeing something else?

/triage needs-user-input

@knative-prow knative-prow bot added the triage/needs-user-input Issues which are waiting on a response from the reporter label Mar 24, 2023
@AswinT22
Copy link
Author

Hey @dprotaso,

Thanks for taking time to check this out.

Indeed, it eventually scales down, but I think the non-active revisions can be scaled down immediately. Am i missing something?

@dprotaso
Copy link
Member

I guess to clarify your ask is

A revision, that has no route pointed to it, should scale down and ignore the configured scale-to-zero-pod-retention-period' ?

Or another way to phrase it

scale-to-zero-pod-retention-period should only apply to revisions that can be reached by a route

This issue might be covered by this PR - #13731 but would need to confirm

@dprotaso
Copy link
Member

dprotaso commented Mar 31, 2023 •
edited

/triage accepted

I'm assuming your ask what I've described - if so I agree that revisions that aren't routable should ignore the scale down retention periods.

@knative-prow knative-prow bot added the triage/accepted Issues which should be fixed (post-triage) label Mar 31, 2023
@dprotaso dprotaso added this to the v1.10.0 milestone Mar 31, 2023
@AswinT22
Copy link
Author

AswinT22 commented Apr 3, 2023

I'm assuming your ask what I've described - if so I agree that revisions that aren't routable should ignore the scale down retention periods.

You are right and let me check the PR

@AswinT22
Copy link
Author

AswinT22 commented Apr 3, 2023 •
edited

I guess to clarify your ask is

A revision, that has no route pointed to it, should scale down and ignore the configured scale-to-zero-pod-retention-period' ?

Or another way to phrase it

scale-to-zero-pod-retention-period should only apply to revisions that can be reached by a route

This issue might be covered by this PR - #13731 but would need to confirm

Hey @dprotaso
I checked the PR, and as far as I am aware it is not covered. Will start working on it once you confirm.

@AswinT22
Copy link
Author

/remove-triage needs-user-input

@knative-prow knative-prow bot removed the triage/needs-user-input Issues which are waiting on a response from the reporter label Apr 20, 2023
@AswinT22
Copy link
Author

@dprotaso, can I go ahead with the fix ?

@dprotaso dprotaso modified the milestones: v1.10.0, v1.11.0 Apr 24, 2023
@dprotaso
Copy link
Member

@AswinT22 sure

/assign @AswinT22

@dprotaso
Copy link
Member

@AswinT22 are you still working on this?

@dprotaso dprotaso modified the milestones: v1.11.0, v1.12.0 Aug 16, 2023
@dprotaso
Copy link
Member

/unassign @AswinT22

@dprotaso dprotaso modified the milestones: v1.12.0, v1.13.0 Nov 21, 2023
@eddy-oum
Copy link

@dprotaso hi can I work on this?

@eddy-oum eddy-oum linked a pull request Apr 25, 2024 that will close this issue
Open
@dprotaso dprotaso modified the milestones: v1.13.0, v1.15.0 May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eddy-oum eddy-oum

Labels
kind/bug Categorizes issue or PR as related to a bug. triage/accepted Issues which should be fixed (post-triage)
Projects
Serving Milestones
Status: No status
Milestone
v1.15.0
Development

Successfully merging a pull request may close this issue.

if revision is inactive, scale to zero instead of waiting for last pod retention eddy-oum/serving
3 participants
@dprotaso @AswinT22 @eddy-oum