From 85843e6e025983ac18f083428dd8c4d475cc8e1c Mon Sep 17 00:00:00 2001 From: matte21 Date: Mon, 28 Oct 2019 14:12:22 +0100 Subject: [PATCH 1/3] Clarify that OnUpdate can mask delete and recreate Kubernetes-commit: ff543ddfc09adf3d0abdf42f9d9fbd57c3ab4b43 --- tools/cache/shared_informer.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tools/cache/shared_informer.go b/tools/cache/shared_informer.go index f59a0852..5c2dcea0 100644 --- a/tools/cache/shared_informer.go +++ b/tools/cache/shared_informer.go @@ -106,7 +106,18 @@ import ( // and index updates happen before such a prescribed notification. // For a given SharedInformer and client, the notifications are // delivered sequentially. For a given SharedInformer, client, and -// object ID, the notifications are delivered in order. +// object ID, the notifications are delivered in order. Because +// `ObjectMeta.UID` has no role in identifying objects, if an object +// O1 with ID (e.g. namespace and name) X and `ObjectMeta.UID` U1 in +// the SharedInformer's local cache is deleted and shortly after +// another relevant object O2 with ID X and `ObjectMeta.UID` U2 is +// created a client has no guarantee of seeing O1's deletion and +// O2's creation as two separate notifications, it might as well +// receive a single update notification where the old object is O1 +// and the new object is O2. Clients that need to detect such cases +// might do so by comparing the `ObjectMeta.UID` field of the old +// and the new object in the code that handles update notifications +// (i.e. `OnUpdate` method of ResourceEventHandler). // // A client must process each notification promptly; a SharedInformer // is not engineered to deal well with a large backlog of From ce6197e865c908d74ae757cad88af1c47a4078c0 Mon Sep 17 00:00:00 2001 From: matte21 Date: Mon, 28 Oct 2019 14:23:37 +0100 Subject: [PATCH 2/3] Fix error in periodic resyncs description Make it clear that periodic resyncs fire update notifications, not create notifications as the old comments incorrectly stated. Kubernetes-commit: 3ae8c864de24a2ad42321c9cbd71099a2b6dcef3 --- tools/cache/shared_informer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/cache/shared_informer.go b/tools/cache/shared_informer.go index 5c2dcea0..f8c1507b 100644 --- a/tools/cache/shared_informer.go +++ b/tools/cache/shared_informer.go @@ -70,7 +70,7 @@ import ( // The local cache starts out empty, and gets populated and updated // during `Run()`. // -// As a simple example, if a collection of objects is henceforeth +// As a simple example, if a collection of objects is henceforth // unchanging, a SharedInformer is created that links to that // collection, and that SharedInformer is `Run()` then that // SharedInformer's cache eventually holds an exact copy of that @@ -140,7 +140,7 @@ type SharedInformer interface { AddEventHandler(handler ResourceEventHandler) // AddEventHandlerWithResyncPeriod adds an event handler to the // shared informer using the specified resync period. The resync - // operation consists of delivering to the handler a create + // operation consists of delivering to the handler an update // notification for every object in the informer's local cache; it // does not add any interactions with the authoritative storage. AddEventHandlerWithResyncPeriod(handler ResourceEventHandler, resyncPeriod time.Duration) From 7d44382a3cb3a4c92dd3f81095285ecbea7c4634 Mon Sep 17 00:00:00 2001 From: matte21 Date: Tue, 31 Dec 2019 09:48:28 +0100 Subject: [PATCH 3/3] Reword modifications for clarity Kubernetes-commit: 21f59c99232afeae05dcdb4f2d1d156cda6a8a77 --- tools/cache/shared_informer.go | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/tools/cache/shared_informer.go b/tools/cache/shared_informer.go index f8c1507b..44ddfdff 100644 --- a/tools/cache/shared_informer.go +++ b/tools/cache/shared_informer.go @@ -107,17 +107,15 @@ import ( // For a given SharedInformer and client, the notifications are // delivered sequentially. For a given SharedInformer, client, and // object ID, the notifications are delivered in order. Because -// `ObjectMeta.UID` has no role in identifying objects, if an object -// O1 with ID (e.g. namespace and name) X and `ObjectMeta.UID` U1 in -// the SharedInformer's local cache is deleted and shortly after -// another relevant object O2 with ID X and `ObjectMeta.UID` U2 is -// created a client has no guarantee of seeing O1's deletion and -// O2's creation as two separate notifications, it might as well -// receive a single update notification where the old object is O1 -// and the new object is O2. Clients that need to detect such cases -// might do so by comparing the `ObjectMeta.UID` field of the old -// and the new object in the code that handles update notifications -// (i.e. `OnUpdate` method of ResourceEventHandler). +// `ObjectMeta.UID` has no role in identifying objects, it is possible +// that when (1) object O1 with ID (e.g. namespace and name) X and +// `ObjectMeta.UID` U1 in the SharedInformer's local cache is deleted +// and later (2) another object O2 with ID X and ObjectMeta.UID U2 is +// created the informer's clients are not notified of (1) and (2) but +// rather are notified only of an update from O1 to O2. Clients that +// need to detect such cases might do so by comparing the `ObjectMeta.UID` +// field of the old and the new object in the code that handles update +// notifications (i.e. `OnUpdate` method of ResourceEventHandler). // // A client must process each notification promptly; a SharedInformer // is not engineered to deal well with a large backlog of