Delete unused status keys from watchable

[arkodg]

Description:

The status keys are only stored and not deleted. Either all the status keys or at least the stale ones should be deleted to free up memory

gateway/internal/gatewayapi/runner/runner.go

Line 112 in cf46fbe

r.ProviderResources.GatewayStatuses.Store(key, &gateway.Status)

&

gateway/internal/xds/translator/runner/runner.go

Line 99 in cf46fbe

r.ProviderResources.EnvoyPatchPolicyStatuses.Store(key, e.Status)

[arkodg]

one solution could be deleting the status from watchable right after processing it in

gateway/internal/provider/kubernetes/status.go

Line 26 in cf46fbe

func (r *gatewayAPIReconciler) subscribeAndUpdateStatus(ctx context.Context) {

[ShyunnY]

At least when we can determine the status update result, we can delete it in subscribeAndUpdateStatus

[arkodg]

yeah +1 @ShyunnY

[ShyunnY]

gateway/internal/status/status.go

Lines 76 to 102 in 1f5df35

	if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
	obj := update.Resource
	// Get the resource.
	if err := u.client.Get(context.Background(), update.NamespacedName, obj); err != nil {
	if kerrors.IsNotFound(err) {
	return nil
	}
	return err
	}
	newObj := update.Mutator.Mutate(obj)
	if isStatusEqual(obj, newObj) {
	u.log.WithName(update.NamespacedName.Name).
	WithName(update.NamespacedName.Namespace).
	Info("status unchanged, bypassing update")
	return nil
	}
	newObj.SetUID(obj.GetUID())
	return u.client.Status().Update(context.Background(), newObj)
	}); err != nil {
	u.log.Error(err, "unable to update status", "name", update.NamespacedName.Name,
	"namespace", update.NamespacedName.Namespace)
	}

When status is updated, do we need to obtain the update result to decide whether to delete status keys?

I have two ideas:

• First: regardless of the status update result, we delete the status key in the watchable. Assuming that the update result fails, there is no need to process it, just wait for the next reconcile. We can delete the status key directly in the subscribeAndUpdateStatus function.
• Second: We modify the apply method to accept err externally. If the status update result err != nil, we skip the deletion of status keys. If everything is fine (which is what we want to see), then we delete the status keys

[arkodg]

@ShyunnY 2 options I see are

• Mark and Sweep in in the publisher Gateway API Layer similar to what is done for XdsIR and InfraIR
  

  gateway/internal/gatewayapi/runner/runner.go

  Line 168 in cf46fbe

  // getIRKeysToDelete returns the list of IR keys to delete

  • Pros

    • status cached in watchable will prevent any more publishes to consumer saving CPU

  • Cons

    • status in watchable takes up memory

• Delete all after processing is done in the subscriber provider layer
  

  gateway/internal/provider/kubernetes/status.go

  Line 77 in cf46fbe

  }),

  • Pros

    • Reduces memory

  • Cons

    • provider will receive all publishes, and will perform an extra read on the resource to compare status, that resource should hopefully be cached in controller-runtime, else it will be an I/O op to API server

[ShyunnY]

In my opinion, I would vote for the first way.

[arkodg]

1 sounds good

[uniglot]

Hi, can I work on this issue with the first option, unless someone's already dealing with it?

[zirain]

@uniglot thanks for taking this.