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

Remove GET job and retries for status updates #105214

Merged
merged 1 commit into from Sep 27, 2021

Conversation

@alculquicondor
Copy link
Member

@alculquicondor alculquicondor commented Sep 23, 2021

What type of PR is this?

/kind bug
/kind flake

What this PR does / why we need it:

Remove GET job and retries for status updates

Also, in the case of conflict, we know that there was a Job update that would trigger another sync, so there is no need to do a rate limited requeue.

Doing a GET right before retrying has 2 problems:

  • It can masquerade conflicts
  • It adds an additional delay

As for retries, we are better of going through the sync backoff.

/sig apps
/area workload-api/job

Which issue(s) this PR fixes:

Fixes #105199

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fix job controller syncs: In case of conflicts, ensure that the sync happens with the most up to date information. Improves reliability of JobTrackingWithFinalizers.
Doing a GET right before retrying has 2 problems:
- It can masquerade conflicts
- It adds an additional delay

As for retries, we are better of going through the sync backoff.

In the case of conflict, we know that there was a Job update that would trigger another sync, so there is no need to do a rate limited requeue.
@alculquicondor
Copy link
Member Author

@alculquicondor alculquicondor commented Sep 23, 2021

/test pull-kubernetes-integration (to spot any flakiness)
/test pull-kubernetes-e2e-gce-ubuntu-containerd

Loading

@alculquicondor
Copy link
Member Author

@alculquicondor alculquicondor commented Sep 24, 2021

/assign @soltysh

Loading

Copy link
Contributor

@soltysh soltysh left a comment

/triage accepted
/priority important-soon
/lgtm
/approve

Loading

@k8s-ci-robot
Copy link
Contributor

@k8s-ci-robot k8s-ci-robot commented Sep 27, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Loading

@k8s-ci-robot k8s-ci-robot merged commit aec9acd into kubernetes:master Sep 27, 2021
14 checks passed
Loading
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment