Skip to content
This repository has been archived by the owner on Mar 12, 2024. It is now read-only.

Shutdown client resources automatically #30

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vemv
Copy link

@vemv vemv commented Aug 7, 2021

This is more abstract/handy, as otherwise one has to keep track of more state in one's application.

I verified that these changes work in a real application; starting and stopping custom client resources works as intended (notably, by using {:nb-io-threads 1 :nb-worker-threads 1} which makes it easy to track if these are being effectively cycled).

Cheers - V

This is more handy, as otherwise one has to keep track of more state in one's application.
@vemv vemv marked this pull request as draft August 7, 2021 02:07
@vemv
Copy link
Author

vemv commented Aug 7, 2021

Uhm, after further testing my solution doesn't work properly - threads leak instaed of being shut down. Even when destroy-client-resource is being properly called.

One thing I noticed is that destroy-client-resource returns a Future, Even when derefing it, the problem persists.

Any ideas?

@lerouxrgd
Copy link
Owner

This is more abstract/handy, as otherwise one has to keep track of more state in one's application.

As you have to keep track of some state like the connector and commands anyway, I think it's not really necessary to try to hide the optional client-resources.

Moreover, client-resources can be shared with some other classes and should not be shutdown automatically by celtuce.

@vemv
Copy link
Author

vemv commented Aug 11, 2021

In the application I was working on I found it really awkward though to pass around a reference to the client-resources from defn to defn. A bit hard to explain without showing the code.

In any case, I was trying to accomplish something fairly simple: have Lettuce create fewer threads (currently it creates quite a lot - not ideal for a laptop). In most libraries one just specifies {:threads 4} or such.

Here I have to create an object, keep track of it separately from Celtuce, and then shut it down. Even when I tried to shut it down I don't think I really accompished it (as I observed threads being leaked).

tldr this seems a bit harder / more error-prone than it should be, and therefore might be a good candidate to be abstracted away.

...Maybe optionally, for keeping the Celtuce lib unopinionated.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants