-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
add count for reuse of each connection and track connection 'freshness' #556
Conversation
Hello @brianc, was wondering if you thought this change was reasonable? If not, I'd need to look for an alternative solution to my particular need. |
return cb(null, client); | ||
}); | ||
}, | ||
destroy: function(client) { | ||
client._destroying = true; | ||
client.__pool_count = undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you use two underscores here? Any special reason for this? Otherwise I would use one to be consistent.
Apart from my comment this looks good to me and is a useful addition. I think we should merge this. @brianc what do you think? |
Well, even better would be no underscore at all and making it part of the official "interface" ^_^ I'll go with whatever you prefer. |
Any comments on this @brianc? |
Been thinking about this. I think it makes okay sense. Really I'd like the pool.create function to be more extensible so this isn't needed and instead someone can supply their own create function and do whatever they want to their clients. But this is a pretty straight forward change and doesn't hurt anything so I'm cool with it. 👍 One thing though...i'd definitely drop the underscores. I usually use underscores to mark things as private: might change, do not touch. Since this is a client facing feature it's cool to just add it to the 'official' interface. |
done, squashed into one commit. |
awesome. thank you so much! I really hate to be a stickler about this kinda thing, but I never use underscored property names in this code base. Everything is |
done, sorry again, should have figured that out myself! |
Prepared a bit of doc for after the release: |
Do you want me to make any other changes to this CL? |
awesomeness! I'll get this merged and released in short order. :) |
add count for reuse of each connection and track connection 'freshness'
add an internal variable to each client to track how many times the client was reused from the pool.
This has the side-benefit that we can now test to see if the client is a "new" connection and take action on a per connection basis.
For example, plv8 initialization function (called for each new GUC) is set on a per connection basis (before any calls to any plv8 functions) as follows:
the client can create an init function:
which is then called on our promise generating connect closure:
Alternatively, I've thought of adding a new event created that would be propagated from pool -> pg but that doesn't really fit with having to wait for the custom initialization to execute before continuing. Adding hooks to add initialization functions to pg would also unnecessarily complicate the code IMO.