feat: add fifo option for pool connection acquisition#4
Open
tgriesser wants to merge 1 commit into
Open
Conversation
tgriesser
commented
Jul 9, 2026
| log?: (msg: string) => any; | ||
| validate?: (resource: T) => boolean; | ||
| propagateCreateError?: boolean; | ||
| fifo?: boolean; |
Member
Author
There was a problem hiding this comment.
Actually went to use this via patch-package somewhere and noticed a warning:
Pool config option "fifo" is no longer supported. See https://github.com/Vincit/tarn.js for possible pool config options.
Due to checking for options from the old pooling and warning
So maybe better to go with the more explicit freeResourceSelectionAlgorithm
Collaborator
There was a problem hiding this comment.
More explicit definitely better here.
Personally,
I'm vibing more with "idle" than "free". "free" is an overloaded term that can confuse - in this context, to the point where someone thinks this is related to the freeing of resources.
And with "strategy" more than "algorithm".
Collaborator
There was a problem hiding this comment.
We could later, if necessary, allow passing a custom strategy, or add more built-ins like "round-robin".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm not sure I ever thought about it critically, but the current approach tarn takes with connection reuse is LIFO. This means that the most recent connections in the stack stay "hot" and get reused again and again, while the earliest created connections will likely not get used and after awhile and will be removed via the idleTimeout reaper.
This is a good thing if you want to keep the number of connections to a minimum, however it's not as helpful if you want to ensure resources stick around for a bit, so that you have enough available for bursts of traffic. Really depends on whether you value keeping the number of active resources low or the frequency of disconnects/reconnects low.
This PR adds a new option
fifo, defaulting tofalse(current behavior), with the option to set totrueto pull from the front of the "free" resource queue. This should also help with situations where there's a set number of "min" resources and some of them timeout due to inactivity during periods of relatively low query volume, because all of the requests go to the most recently used items.Would also be fine with naming the option something more explicit like is mentioned in the issue below:
Related: Vincit/tarn.js#76