Skip to content
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.

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

huge memory usage #4516

Open
qicz opened this issue Oct 24, 2024 · 1 comment · May be fixed by #4527
Open

huge memory usage #4516

qicz opened this issue Oct 24, 2024 · 1 comment · May be fixed by #4527

Comments

@qicz
Copy link
Member

qicz commented Oct 24, 2024

follow #4181

The resource cached many resources. Any resource that changes will send a signal, and runners will subscribe to the change message. There are many DeepCopy.

I think that we should provide a global cache resource mapping, and the runners subscribe to the simple change signal. in the HandleSubscription function or Translator gets the resource in the global cache.

image

image

for the route status, the DeepCopy uses a huge memory. Can we remove this DeepCopy?for my analysis to compare the status change we just cmp the val.Parents and obj.Status.Parents. or move the data to the global cache?

like follows:

  • remove DeepCopy and CMP the status directly
// HTTPRoute object status updater
go func() {
	message.HandleSubscription(r.resources.HTTPRouteStatuses.Subscribe(ctx),
		func(update message.Update[types.NamespacedName, *gwapiv1b1.HTTPRouteStatus]) {
			// skip delete updates.
			if update.Delete {
				return
			}
			key := update.Key
			val := update.Value
			r.statusUpdater.Send(status.Update{
				NamespacedName: key,
				Resource:       new(gwapiv1b1.HTTPRoute),
				Mutator: status.MutatorFunc(func(obj client.Object) bool {
					h, ok := obj.(*gwapiv1b1.HTTPRoute)
					if !ok {
						panic(fmt.Sprintf("unsupported object type %T", obj))
					}
					// remove DeepCopy
					// hCopy := h.DeepCopy() 
					// hCopy.Status.Parents = val.Parents
					// cmp the Parents
					t := gwapiv1b1.HTTPRouteStatus{}
					t.Parents = val.Parents
					if isStatusEqualHTTPRoute(obj.Status, t) {
						u.log.WithName(h.NamespacedName.Name).
						WithName(h.NamespacedName.Namespace).
						Info("status unchanged, bypassing update")
						return true
					}
					h.Status.Parents = val.Parents
					return false
				}),
			})
		},
	)
	r.log.Info("httpRoute status subscriber shutting down")
}()

func isStatusEqualHTTPRoute(a gwapiv1b1.HTTPRouteStatus, b gwapiv1b1.HTTPRouteStatus) bool {
	opts := cmp.Options{
		cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
		cmpopts.IgnoreMapEntries(func(k string, _ any) bool {
			return k == "lastTransitionTime"
		}),
	}
	if cmp.Equal(a, b, opts) {
		return true
	}

	return false
}
func (u *UpdateHandler) apply(update Update) {
	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
		}

		if update.Mutator.Mutate(obj) {
			return nil
		}

		return u.client.Status().Update(context.Background(), obj)
	}); err != nil {
		u.log.Error(err, "unable to update status", "name", update.NamespacedName.Name,
			"namespace", update.NamespacedName.Namespace)
	}
}
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

@github-actions github-actions bot added the stale label Nov 24, 2024
@qicz qicz removed the stale label Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant