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

Spike: Unregister runtime artifacts #2619

Closed
dhmlau opened this issue Mar 21, 2019 · 6 comments
Closed

Spike: Unregister runtime artifacts #2619

dhmlau opened this issue Mar 21, 2019 · 6 comments

Comments

@dhmlau
Copy link
Member

dhmlau commented Mar 21, 2019

Description / Steps to reproduce / Feature proposal

Spike for #2485

Investigate on how to unbind/unregister LB artifacts:

  • Model class
  • Repository class
  • Service proxy class
  • Controller class
  • DataSource class

Questions:

  1. Does this spike include all the LB artifacts? Or if we figure out a smaller subset, it's trivial for the rest?
  2. Does the scope of the spike include the cache management mentioned by @jannyHou in this comment?
@dhmlau dhmlau added spike Core @loopback/core labels Mar 21, 2019
@bajtos bajtos added 2019Q2 and removed 2019Q2 labels Apr 11, 2019
@raymondfeng
Copy link
Contributor

We now have the infrastructure in place, such as ContextView, @inject.view, and @inject.getter. The key is to extract certain logic into extension points and utilize the dynamic access to registered extensions. For example, the RestServer should be able to react on controller come and go.

@raymondfeng
Copy link
Contributor

We also need to find out which artifacts may have impacts when they are changed. It also has something to do with binding scopes.

@bajtos
Copy link
Member

bajtos commented Jun 11, 2019

Does the scope of the spike include the cache management mentioned by @jannyHou in this comment?

Yes. Removing a controller must remove the REST API endpoints too.

Does this spike include all the LB artifacts? Or if we figure out a smaller subset, it's trivial for the rest?

I think different artifacts may need different approach. For example, there may be very little work needed to unregister a model class. On the other hand, removing a controller is more involved, as we have to update the routing table.

It may be better to split this spike into smaller tasks, for example:

  1. Controller class
  2. Repository and Service proxy
  3. Model class
  4. DataSource class

I think we need to take into account dependencies between artifacts too. For example, it does not make sense to unregister a Repository class when there are controllers still using it.

In the first iteration, I am fine to leave it up to developers to ensure they unregister all artifact types. We can investigate more robust solutions later.

The key requirements for the first iteration:

  • When a controller + repo + model is unregistered, there must be no memory references remaining, so that V8 can release and collect the memory. Test case: write a loop creating & unregistering model+repo+controller. The process should use relatively constant memory, the memory usage must not grow.
  • Similarly for controller + service proxy.
  • Similarly for data-sources.

@stale
Copy link

stale bot commented Sep 7, 2020

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

@stale stale bot added the stale label Sep 7, 2020
@dhmlau dhmlau removed the stale label Sep 7, 2020
@stale
Copy link

stale bot commented Jul 14, 2021

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

@stale stale bot added the stale label Jul 14, 2021
@stale
Copy link

stale bot commented Aug 13, 2021

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

@stale stale bot closed this as completed Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants