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

feat: add dump function for current connections count #2562

Closed
wants to merge 1 commit into from

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Jul 27, 2023

No description provided.

@fengmk2 fengmk2 requested a review from dougwilson July 27, 2023 10:28
@sidorares
Copy link
Member

I'm probably +1 to expose this information somehow
At the same time I'd prefer to minimise surface api, since queryingConnectionsCount can be computed from other 3 maybe not worth adding

Also re naming: instead dumpConnectionsCount - maybe something like connectionsInformation or getConnectionsInformation. Also maybe idleConnectionsCount instead of freeConnectionsCount - need to check if related information is exposed under similar name

@sidorares
Copy link
Member

looks like CI config need some refresh, potentially dropping node 0.10 and moving appveyor to actions windows runner

@fengmk2
Copy link
Member Author

fengmk2 commented Jul 27, 2023

cc @qile222

@dougwilson
Copy link
Member

dougwilson commented Jul 27, 2023

Well, we don't want to remove node.js versions from the CI that are supported by the current version. Usually removing support is a semver major action. Taking a look, the CI appveypr seem to be fine, just the new code in the PR needs to be adjusted to actually work in Node.js 0.10 is all (I suspect it is the use of deepStrictEqual in the tests as I don't think thay existed then). And I will update gh actions runner host for those.

As for the change, it seems fine, though whay about just calling it getConnnectionCounts() or such?

@dougwilson
Copy link
Member

Also, I think maybe queryingConnectionsCount is going to.be confusing, since the math for thay number is not the number of connections making a query, but just like the connection that are checked out from the pool, even if they are just idle.

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

Successfully merging this pull request may close these issues.

4 participants