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.

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

Configure sane default value for grpc_next_upstream_tries #12090

Open
mattb18 opened this issue Oct 2, 2024 · 3 comments
Open

Configure sane default value for grpc_next_upstream_tries #12090

mattb18 opened this issue Oct 2, 2024 · 3 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@mattb18
Copy link

mattb18 commented Oct 2, 2024

I'd like ingress-nginx to set a sane default value for grpc_next_upstream_tries. There is currently no value set for this directive, meaning that the default nginx value is used, which in this case is zero. Ideally, this value should also be configurable via an annotation.

I've recently experienced this issue, which resulted in an infinite retry (every 5 seconds due to timeouts) to an upstream that was removed mid request. To make it worse, this turned into an infinite retry with no timeout once the IP was used by another pod which was not listening on the corresponding port, resulting in millions of requests attempts a minute.

As per the ticket, configuring grpc_next_upstream_tries to a sane value (3 in my case) resolves this issue, as the request is tried only 3 times before giving up, rather than retrying infinitely.

A similar change was made for proxy_next_upstream_tries in #6553, to resolve #5425.

Would it make sense to also set grpc_next_upstream_tries to 3 or another similar value?

Alternatively, it would be good to set the default to 0, and allow configuring via an annotation (to prevent any potential breaking changes). This way we can avoid needing to configure server snippets annotations.

I'm happy to create a PR if the consensus for this requests is positive.

@mattb18 mattb18 added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 2, 2024
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Oct 2, 2024
@mattb18 mattb18 changed the title Configure sane defaullt values for grpc_next_upstream_tries Configure sane default value for grpc_next_upstream_tries Oct 2, 2024
@siddheshgarud
Copy link

/assign

@giner
Copy link

giner commented Oct 31, 2024

Here is a Terraform snippet with a temporary workaround for nginx ingress on MicroK8s

resource "kubernetes_config_map_v1_data" "ingress_workaround" {
  metadata {
    namespace = "ingress"

    name = "nginx-load-balancer-microk8s-conf"
  }

  data = {
    "http-snippet" = "grpc_next_upstream_tries 3;"
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

No branches or pull requests

4 participants