You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
Currently, the MysqlConnectionOptions poolSize option specifies the connection limit for all replica connections.
In this case, you cannot specify a connection limit for each master or slave.
For example, if you only want to create one master connection and up to 10 slave connections, Typeorm has no solution.
I added the MysqlConnectionCredentialsOptions poolSize option, allowing you to specify a poolSize for each replica.
Verification Method
If the master's poolSize is 1 and the `slaves[0] poolSize is 2,
then the master connection should be open at most 1, and the slaves[0] connection should be open at most 2.
Pull-Request Checklist
[o ] Code is up-to-date with the master branch
This pull request links relevant issues as Fixes #00000
There are new or updated unit tests validating the change
Documentation has been updated to reflect this change
The PR adds a new feature allowing per-connection poolSize configuration, but no unit tests are included to verify the behavior. Tests should validate that credential-level poolSize overrides global poolSize, that fallback to global poolSize works when not specified, and that zero values are handled correctly.
The comment 'for each connection' on line 49 is unclear and grammatically incomplete. It should be rephrased to clearly explain that this poolSize setting applies to the specific connection credential and overrides the global poolSize option when provided.
/** * Maximum number of clients the pool should contain. * for each connection */
readonly poolSize?: number
Why: The suggestion correctly identifies that an invalid poolSize (e.g., zero or negative) could cause connection pool issues, and proposes adding a check to ensure it is a positive number, which improves the robustness of the configuration.
Validate that the poolSize from credentials is a positive number before assigning it to connectionLimit. If the validation fails, fall back to the poolSize from options.
Why: The suggestion correctly identifies that poolSize should be a positive number and adds validation to prevent potential connection pool issues from invalid configuration, improving the code's robustness.
Validate that credentials.poolSize is a positive number before using it for the connectionLimit, falling back to options.poolSize if the validation fails.
Why: The suggestion correctly identifies that a non-positive poolSize could cause issues and proposes a valid check, improving the robustness of the configuration handling.
Replace the nullish coalescing operator (??) with the logical OR operator (||) to handle the poolSize configuration. This prevents a poolSize of 0 from being passed to the driver, which would cause issues, and instead falls back to the next option or the driver's default.
Why: The suggestion correctly identifies that a poolSize of 0 is problematic for a connection pool and proposes using || instead of ?? to prevent this invalid configuration from being passed to the driver, thus improving the robustness of the connection logic.
Shall we add a bit of documentation and unit test for it?
The documentation for all the MysqlConnectionCredentialsOptions properties is missing. I think we can create another PR to achieve the documentation for these props.
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.
pkuczynski
left a comment
gioboa
left a comment
Description of change
Currently, the
MysqlConnectionOptionspoolSize option specifies the connection limit for all replica connections.In this case, you cannot specify a connection limit for each master or slave.
For example, if you only want to create one master connection and up to 10 slave connections, Typeorm has no solution.
I added the
MysqlConnectionCredentialsOptionspoolSize option, allowing you to specify a poolSize for each replica.Verification Method
If the master's poolSize is 1 and the `slaves[0] poolSize is 2,
then the master connection should be open at most 1, and the slaves[0] connection should be open at most 2.
Pull-Request Checklist
masterbranchFixes #00000Related Issues
#8799
Summary by CodeRabbit