-
-
Notifications
You must be signed in to change notification settings - Fork 10.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
feat: support portal restTemplate Client connection pool config #5200
feat: support portal restTemplate Client connection pool config #5200
Conversation
WalkthroughThe recent changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PortalConfig
participant RestTemplateFactory
participant ConnectionManager
User->>PortalConfig: Request config values
PortalConfig-->>User: Return connectPoolMaxTotal() and connectPoolMaxPerRoute()
User->>RestTemplateFactory: Create RestTemplate with config
RestTemplateFactory->>ConnectionManager: Set connection limits
ConnectionManager-->>RestTemplateFactory: Connection pool ready
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java (2 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java (1 hunks)
Additional comments not posted (4)
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java (2)
64-66
: Ensure connection pool parameters are correctly configured.The
PoolingHttpClientConnectionManager
is correctly initialized and configured usingportalConfig
. This change aligns with the PR objectives to enhance connection management.
70-70
: Verify connection manager integration.The connection manager is correctly set in the
HttpClientBuilder
. Ensure that this integration is thoroughly tested to confirm improved performance under high-load scenarios.Verification successful
Connection Manager Integration Verified
The
RetryableRestTemplateTest.java
file contains several tests that verify the behavior of theRetryableRestTemplate
, which usesRestTemplateFactory
to createRestTemplate
instances. These tests simulate various scenarios, including server availability and request retries, which indirectly test the integration of the connection manager by ensuring theRestTemplate
behaves correctly under different conditions.
- The tests cover scenarios such as:
- Handling of server timeouts and retries.
- Correct handling of HTTP methods (GET, POST, PUT, DELETE).
- Use of access tokens in requests.
These tests provide confidence that the connection manager integration is functioning as expected, particularly in handling network issues and retries.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of connection manager in RestTemplateFactory. # Test: Search for usages of RestTemplateFactory to ensure the connection manager is utilized. rg --type java -A 5 $'RestTemplateFactory'Length of output: 9302
Script:
#!/bin/bash # Search for test cases involving RestTemplateFactory to verify connection manager integration. rg --type java -A 10 'RestTemplateFactory' | grep -i 'test'Length of output: 57
Script:
#!/bin/bash # Search for test files related to components using RestTemplateFactory to verify connection manager integration. rg --type java -l 'ConfigReleaseWebhookNotifier|AdminServiceAddressLocator|RetryableRestTemplate|SystemInfoController' | grep -i 'test'Length of output: 228
Script:
#!/bin/bash # Display the contents of RetryableRestTemplateTest.java to verify connection manager integration. cat apollo-portal/src/test/java/com/ctrip/framework/apollo/portal/RetryableRestTemplateTest.javaLength of output: 20641
apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java (2)
176-178
: Default value for maximum total connections.The method
connectPoolMaxTotal
correctly retrieves the property with a default value of 20. Ensure this default aligns with typical usage scenarios.
180-182
: Default value for maximum connections per route.The method
connectPoolMaxPerRoute
correctly retrieves the property with a default value of 2. Ensure this default aligns with typical usage scenarios.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- CHANGES.md (1 hunks)
Additional comments not posted (1)
CHANGES.md (1)
17-17
: Changelog entry looks good!The new entry in the changelog is clear and provides a useful reference to the pull request for more details.
...lo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java
Outdated
Show resolved
Hide resolved
...lo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/config/PortalConfig.java
Show resolved
Hide resolved
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/component/RestTemplateFactory.java
Please complete the CLA by following the instructions provided by the CLA bot. |
I have read the CLA Document and I hereby sign the CLA |
# Conflicts: # CHANGES.md
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.
Actionable comments posted: 0
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.
LGTM
What's the purpose of this PR
Portal 模块 RestTemplate 支持配置 HTTP最大连接池和每个路由的连接大小 详见 #5199
Which issue(s) this PR fixes:
Fixes #5199
Brief changelog
support portal restTemplate Client connection pool config
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.Summary by CodeRabbit
New Features
Documentation