Skip to content

feat: disable write back informer by default #15079

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

Merged
Joibel merged 5 commits into from
Dec 4, 2025
Merged

feat: disable write back informer by default #15079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7a6f681
feat: disable write back informer by default
eduardodbr Nov 26, 2025
ccc9484
chore: add feature md
eduardodbr Nov 26, 2025
37f1465
Merge branch 'main' into disable-write-back-informer
eduardodbr Nov 26, 2025
e05a4d0
fix: feature component
eduardodbr Nov 28, 2025
a41afb7
Merge branch 'main' into disable-write-back-informer
eduardodbr Nov 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .features/pending/disable-write-back-informer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Description: Disable write back informer by default
Author: [Eduardo Rodrigues](https://github.com/eduardodbr)
Component: General
Issues: 12352

Update the controller’s default behavior to disable the write-back informer. We’ve seen several cases of unexpected behavior that appear to be caused by the write-back mechanism, and Kubernetes docs recommend avoiding writes to the informer store. Although turning it off may increase the frequency of 409 Conflict errors, it should help reduce unpredictable controller behavior.

2 changes: 1 addition & 1 deletion docs/environment-variables.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ This document outlines environment variables that can be used to customize behav
| `EXPRESSION_TEMPLATES` | `bool` | `true` | Escape hatch to disable expression templates. |
| `EVENT_AGGREGATION_WITH_ANNOTATIONS` | `bool` | `false` | Whether event annotations will be used when aggregating events. |
| `GZIP_IMPLEMENTATION` | `string` | `PGZip` | The implementation of compression/decompression. Currently only "`PGZip`" and "`GZip`" are supported. |
| `INFORMER_WRITE_BACK` | `bool` | `true` | Whether to write back to informer instead of catching up. |
| `INFORMER_WRITE_BACK` | `bool` | `false` | Whether to write back to informer instead of catching up. |
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an extra trailing space after false in the default value column. This should be removed for consistency with other entries in the table.

Suggested change
| `INFORMER_WRITE_BACK` | `bool` | `false` | Whether to write back to informer instead of catching up. |
| `INFORMER_WRITE_BACK` | `bool` | `false` | Whether to write back to informer instead of catching up. |

Copilot uses AI. Check for mistakes.
| `HEALTHZ_AGE` | `time.Duration` | `5m` | How old a un-reconciled workflow is to report unhealthy. |
| `INDEX_WORKFLOW_SEMAPHORE_KEYS` | `bool` | `true` | Whether or not to index semaphores. |
| `LEADER_ELECTION_IDENTITY` | `string` | Controller's `metadata.name` | The ID used for workflow controllers to elect a leader. |
Expand Down
7 changes: 4 additions & 3 deletions workflow/controller/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,14 +798,15 @@ func (woc *wfOperationCtx) persistUpdates(ctx context.Context) {
woc.log.WithFields(logging.Fields{"resourceVersion": woc.wf.ResourceVersion, "phase": woc.wf.Status.Phase}).Info(ctx, "Workflow update successful")

switch os.Getenv("INFORMER_WRITE_BACK") {
// By default we write back (as per v2.11), this does not reduce errors, but does reduce
// this does not reduce errors, but does reduce
// conflicts and therefore we log fewer warning messages.
Comment on lines +801 to 802
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is now grammatically incomplete after the partial removal. It starts with lowercase "this does not reduce errors" which makes it unclear what "this" refers to. Consider revising to: "Writing back to the informer does not reduce errors, but does reduce conflicts and therefore we log fewer warning messages."

Suggested change
// this does not reduce errors, but does reduce
// conflicts and therefore we log fewer warning messages.
// Writing back to the informer does not reduce errors, but does reduce conflicts and therefore we log fewer warning messages.

Copilot uses AI. Check for mistakes.
case "", "true":
case "true":
if err := woc.writeBackToInformer(); err != nil {
woc.markWorkflowError(ctx, err)
return
}
case "false":
// no longer write back to informer cache as default (as per v4.0)
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 808 starts with lowercase and lacks proper sentence structure. Consider revising to: "No longer write back to informer cache as default (as per v4.0)."

Suggested change
// no longer write back to informer cache as default (as per v4.0)
// No longer write back to informer cache as default (as per v4.0).

Copilot uses AI. Check for mistakes.
case "", "false":
time.Sleep(1 * time.Second)
}

Expand Down
Loading