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

Expose an API to close channels #769

Closed
merlinnot opened this issue Sep 30, 2019 · 10 comments · Fixed by #824
Closed

Expose an API to close channels #769

merlinnot opened this issue Sep 30, 2019 · 10 comments · Fixed by #824
Assignees
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@merlinnot
Copy link

Is your feature request related to a problem? Please describe.

Running more than one instance of Firestore in isolated, disposable contexts is not possible, as the underlying connection makes it impossible for these clients to be cleared from memory.

An example would be to use Node.js VM.

Describe the solution you'd like

I'd like to see an API to close all open streams, so the client can be garbage collected.

Describe alternatives you've considered

Hacking my way into @grpc/grpc-js, which exposes this API.

Additional context

Here's an example of what it can cause :)

Screen Shot 2019-09-30 at 21 39 01

@bcoe bcoe added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Sep 30, 2019
@schmidt-sebastian
Copy link
Contributor

@bcoe / @BenWhitehead / @JustinBeckwith Thoughts? I am not opposed, especially since we now offer terminate() in the Mobile SDKs.

@JustinBeckwith
Copy link
Contributor

No concerns from me, but I defer to @bcoe and @BenWhitehead

@BenWhitehead
Copy link
Contributor

Sounds like a good thing to me, this also already exists in the Java server sdk via (close).

@schmidt-sebastian schmidt-sebastian self-assigned this Oct 1, 2019
@bcoe
Copy link
Contributor

bcoe commented Oct 1, 2019

@schmidt-sebastian, @BenWhitehead we should loop in @alexander-fenster to this conversation too 😄; there was conversation about functionality along the lines of client.unref() which result in garbage collection as soon as there's no work on the wire.

@merlinnot
Copy link
Author

Thanks for picking it up @schmidt-sebastian! We're also doing some work to make it actually work with @grpc/grpc-js: grpc/grpc-node#1083.

@schmidt-sebastian
Copy link
Contributor

We are currently stuck trying to get the new Typescript generator to play nicely with the repo, but hope to have this resolved early next week. If you are blocked by this, I am not opposed to applying the changes manually to dev/v1/firestore_client.js, but note that this will be temporary.

@merlinnot
Copy link
Author

Hey, thank you for the update, much appreciated. One more week of oversized VMs won’t kill anyone, so take your time 🙂

@merlinnot
Copy link
Author

@schmidt-sebastian Is it now unblocked with changes made in v3? 😃

@schmidt-sebastian
Copy link
Contributor

Yes! @thebrianchen will take care of this.

@merlinnot
Copy link
Author

@thebrianchen @schmidt-sebastian Thank you so much! Feels like I got my Christmas gifts early 😄 On a more scientific side, I reduced my socket usage by 96.875% and memory usage by 1.4 GBs per thread!

@google-cloud-label-sync google-cloud-label-sync bot added the api: firestore Issues related to the googleapis/nodejs-firestore API. label Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants