Storage creates multiple Notifications about knative-gcp HOT 6 CLOSED

/assign @vaikas-google

from knative-gcp.

@Harwayne can you try this flow again with the operations ?

from knative-gcp.

as @n3wscott mentions, this should not happen again. The problem was that occasionally depending on which write (finalizer or the status update with the notificationId in Status) would go first we would indeed end up getting the duplicate notification. With the operations model, creating the subscription should be idempotent because the state is stored in the Job that creates the Subscription and not in the reconciler.

from knative-gcp.

@Harwayne can you still repro this?

from knative-gcp.

Looking through the code, I think it should be much safer now.

However, it is still possible if the Pod creating the notification dies at the wrong time. If the Pod dies after https://github.com/google/knative-gcp/blob/master/pkg/operations/storage/notification.go#L228, but before https://github.com/google/knative-gcp/blob/master/pkg/operations/storage/notification.go#L243, or writing the termination message fails for some reason, then we would still leak a Notification.

One possible solution is to set a custom attribute in the notification to the UID of the Storage object. Then, before creating a Storage notification, list the existing ones and search for that UID. If found, then accept it.

from knative-gcp.

I cannot repro this in 0.12... leaving it open for now, but may close it in 0.13 if nobody can reproduce

from knative-gcp.