Repository navigation
Conversation
Every idle reconcile sent the apiserver a server-side apply for each object the operator manages, DELETEs for objects already gone, a create-then-409 for the current ReplicaSet and a status patch: about 22 requests per RestateCluster every 5 minutes. On EKS each of those is an audit record billed by CloudWatch. Writes now go through OwnedObjects, which fronts each kind with its watch cache. An apply is skipped when this process last applied the same desired object and the cached object is still what that apply produced; any other write to it (spec, labels, annotations, owner references, or any change for kinds without a generation), a cache miss or a restart applies again, so out-of-band drift is still repaired when the watch reports it. Deletes are skipped when the synced cache has no such object, the canary Jobs are no longer re-applied once they have run, and status patches are skipped when the status is unchanged. RestateDeployment looks the current ReplicaSet up in its prewarmed store instead of creating it and handling the 409, and only patches its scale and annotations when they differ. The steady-state resync is now configurable (--resync-interval-seconds, RESYNC_INTERVAL_SECONDS, Helm resyncIntervalSeconds), defaults to 30 minutes, and is jittered by 10% so resources reconciled together after a restart drift apart. NotReady and error requeues are unchanged. The RestateCloudEnvironment watch selected on a label the tunnel objects never carry, so it saw nothing; it now selects on app.kubernetes.io/name=restate-cloud-tunnel-client.
kube-runtime's owns_stream keys an owner by its child's namespace. For the cluster-scoped RestateCluster and RestateCloudEnvironment, every namespaced child therefore queued a second, namespaced key for its owner, which reconciled and requeued alongside the real one: each RestateCluster was reconciled twice per interval. Map children to their owners by name instead.
The tunnel Deployment watch filtered on generation and labels, which a delete event leaves unchanged, so a deleted tunnel Deployment waited for the resync to be recreated. All cached watches now share the reflector helper that bumps the generation on deletion. A ReplicaSet in the store that looks like someone else's may already be gone from the apiserver, for example after the RestateDeployment was deleted and recreated. Treating it as a hash collision would rename the version permanently, so such a cache hit now goes through the create, and only the apiserver's 409 can establish a collision.
The watch caches only hold objects that match their label selector, so a cache miss could hide an object of ours that had lost its labels, and the delete would never be sent. A delete is now skipped only once this process has seen the apiserver report the object gone, and has neither applied it since nor seen it reappear in the cache.
A recorded absence was trusted forever, so an object recreated under one of our names but without our labels, which the label-selected cache cannot see, would never be deleted. Absences now expire after 30 minutes, the default resync, at the cost of one DELETE per absent name per interval.
lukebond
left a comment
lukebond
left a comment
There was a problem hiding this comment.
thank you for addressing this problem. i've known about it and haven't done anything about it, so i appreciate it.
the implementation is correct i think but it's unconventional in the k8s controller space. so i want to push back on this approach and steer to a different solution that will be simpler, less unusual and more idiomatic for a k8s controller.
it's common to re-reconcile on a timer even when a watched resource hasn't changed, but the onus is on the controller to ensure that this is cheap and results in an early exit. the current behaviour blindly applies and deletes objects unnecessarily, so it's failing that and this is what you're fixing. but the more idiomatic way to do it is a spec-hash annotation, and it's what most operators that care about this do (e.g. Flux). something like:
- compute the desired hash
- stamp it on the object as an annotation when you apply
- on reconcile, read the object from the watch cache (no API call) and skip the apply if its stored annotation already equals hash(desired)
the controller machinery (kube-rs in rust, controller-runtime in go) has in-built caching to be leveraged here, no need to roll your own.
your description shows you considered the annotation and rejected it because it "can't tell when someone edited the spec under an unchanged annotation unless you track generations, which brings back in-memory state." i think that's misundertood: 1. external edits to a child are caught by the watch; the change event re-triggers a reconcile and you re-apply, so the annotation never has to detect drift itself. 2. generation tracking doesn't need in-memory state: for the CRs it belongs in status.observedGeneration, the conventional persisted field, and the guard is observedGeneration == metadata.generation. pair that with status predicates on the child watches and an unchanged resource does no work at all- no per-object cache, and it survives restarts (yours re-applies everything on restart). the spec-hash annotation then handles the secondary case of skipping an unchanged child.
|
:100 -- hard agree with @lukebond |
Summary
An idle RestateCluster made 21 Kubernetes API requests per reconcile and was reconciled twice every 5 minutes, about 12,000 requests (18 MB of audit log) per cluster per day. Almost all of them were server-side applies that changed nothing, plus deletes of objects that were already gone. On EKS every request is an audit record that CloudWatch bills per GB. With this change an idle reconcile makes no API requests (apart from a recheck every 30 minutes of objects that should be absent), each resource is reconciled once per interval, and the default interval is 30 minutes.
Measured on kind with apiserver audit logging (method under Validation):
Two per-cluster costs remain:
Approach and decisions
src/controllers/owned_objects.rs). Each kind the operator writes has anOwnedObjects<K>backed by its watch store. It remembers, in memory, a hash of the last object it applied to each name and a fingerprint of the object that apply returned. An apply is skipped only when both still match the cache. The fingerprint is uid + generation + labels + annotations + owner references, or the resourceVersion for kinds without a generation. So any spec or metadata write by someone else, a delete, a cache miss or an operator restart means applying again, and drift is repaired as soon as the watch delivers the change. Status-only writes don't count, so a busy status doesn't cause re-applies.spec.replicasdiffers. It patches annotations only when a value differs, or when thepropagate-annotationsfield manager owns a different set of keys (read frommanagedFields). The 409 path stays as the fallback for a stale store. A cached ReplicaSet that looks like a hash collision also goes through that path, so only the apiserver's answer can bumpcollisionCount.owns_streamkeys an owner by its child's namespace (ObjectRef::from_owner_ref(ns, …), kube-runtime 2.0.1controller/mod.rs:231). For the cluster-scoped RestateCluster and RestateCloudEnvironment, that queued a second key (rc-a.rc-a), which reconciled and requeued alongside the real one. Children are now mapped to their owner by name withwatches_stream.--resync-interval-seconds/RESYNC_INTERVAL_SECONDS/ HelmresyncIntervalSeconds. Default 1800 s, jittered ±10% on every requeue so resources reconciled together after a restart drift apart.Impact
restate.dev/owned-bylabel. Edits to them used to wait for the next requeue; they are now reverted immediately.app.kubernetes.io/managed-by=restate-operator, a label the tunnel objects never carry, so it saw nothing. It now selects onapp.kubernetes.io/name=restate-cloud-tunnel-client.resyncIntervalSecondsif that matters.Validation
cargo test --lib(148 passed) andcargo clippy --all-targetsare clean. A new unit test checks thatOwnedObjectsskips an unchanged apply, re-applies after an out-of-band edit, and skips a delete the synced cache says is absent.cargo test --dochas 5 failures that are already on main (doc comments inpodidentityassociations.rsandquantity_parser.rs).e2e/rollback.shpassed 13/13. One earlier full run failed a single assertion in thepinned-newscenario ("v2 never became latest"). At that point Restate was already routing to v2. The likely cause is a race in the script:rd_readyaccepts the staleReady=Trueleft from v1, so v2's deployment id can be read before its ReplicaSet exists. That scenario passed when run on its own, and on a full rerun.omitStages: [RequestReceived]and a file backend.resyncIntervalSeconds: 30.restateService: reverted in 0.2 s (main: 0.2 s).targetPort: reverted in under 1 s (main: only at the next requeue).publishNotReadyAddresses, are left alone, as on main (server-side apply semantics).collisionCount.gpt-6.1-sol, thengpt-6-astra) found that skipping deletes on a cache miss alone could leave behind an object that had lost our labels. It then found that trusting a recorded absence forever had the same problem for an object recreated without them. The recheck described above fixes both. Unit tests and clippy pass after these fixes; e2e last ran before them.Claude (Opus 5.5) built the measurement harness, ran both builds, and traced the double reconcile to kube-runtime's
owns_stream.