feat(configservice): Enhanced instance configuration auditing and caching#5361
Conversation
dosubot
bot
added
the
size:M
WalkthroughThis change introduces new configurable parameters in the configuration management classes. The Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
dosubot
bot
added
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/en/deployment/distributed-deployment-guide.md (1)
1636-1665: Review of New Configuration Documentation (Sections 3.2.14–3.2.17)The newly introduced sections detailing the following configuration parameters are clear and cohesive:
- 3.2.14 instance.config.audit.max.size: Describes the maximum queue size for audit records with specified default (10000) and minimum (10) values.
- 3.2.15 instance.cache.max.size: Details the maximum number of instance caches with a default of 50000 and a minimum of 10.
- 3.2.16 instance.config.cache.max.size: Specifies limitations for instance config caches identically to section 3.2.15.
- 3.2.17 instance.config.audit.time.threshold.minutes: Explains the interval threshold (default 10, minimum 5 minutes) for saving/updating audit records.
Each section clearly states the respective default and minimum values and reminds the user that a service restart is required for the changes to take effect.
Suggestions:
- Consistent Restart Reminder: The phrase “After the modification, you need to restart for it to take effect” is clear; however, consider rephrasing to a more concise variant such as “A restart is required for changes to take effect” for consistency and brevity across the document.
- Impact Note (Optional): Adding a brief note on potential impacts—if any—on performance or caching behavior when modifying these parameters could further improve the documentation.
Overall, these additions enhance the usability of the deployment guide by providing detailed control over audit logging and cache management in the latest version (2.5.0 and above).
docs/zh/deployment/distributed-deployment-guide.md (4)
1575-1580: Documentation Update: Instance Config Audit Max Size Parameter
The new parameterinstance.config.audit.max.sizeis clearly documented with a default value of 10,000 and a minimum of 10. It effectively conveys that exceeding this queue size will result in discarding the oldest audit records. Consider explicitly stating that the value refers to the number of audit records and emphasize that any modifications require a service restart.
1581-1586: Documentation Review: Instance Cache Max Size Parameter
The parameterinstance.cache.max.sizeis specified clearly with a default of 50,000 and a minimum of 10. It explains that when the cached instances exceed this limit, an eviction mechanism is triggered. For enhanced clarity, consider adding a brief note about the eviction strategy (e.g., whether it uses FIFO, LRU, etc.) if that information is available.
1587-1592: Documentation Review: Instance Config Cache Max Size Parameter
This parameter,instance.config.cache.max.size, is well described with a default of 50,000 and a minimum of 10, detailing its role in controlling the maximum number of cached instance configurations. As with the previous parameter, a short note on the details of the eviction mechanism could further aid users in understanding its behavior during cache overflow.
1593-1606: Documentation Update: Instance Config Audit Time Threshold Parameter
The new parameterinstance.config.audit.time.threshold.minutesis clearly introduced with a default value of 10 minutes and a minimum of 5 minutes. It succinctly explains that only if the interval between two configuration-pull requests exceeds this threshold will the audit record be saved or updated. For clarity, please specify whether the comparison is inclusive (i.e., if the interval is exactly 10 minutes) or exclusive.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/en/deployment/distributed-deployment-guide.md(1 hunks)docs/zh/deployment/distributed-deployment-guide.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (8)
...ice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java
Outdated
Show resolved
Hide resolved
...ice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java
Outdated
Show resolved
Hide resolved
...ice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java(5 hunks)apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtilTest.java(3 hunks)docs/zh/deployment/distributed-deployment-guide.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (8)
🔇 Additional comments (14)
apollo-configservice/src/test/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtilTest.java (4)
19-19: Added required imports for new dependencies.The new imports for
BizConfigandMeterRegistryare necessary to support the enhanced instance configuration auditing and caching functionality.Also applies to: 23-23
46-49: Added mock objects for new dependencies.The test correctly adds mock objects for
BizConfigandMeterRegistry, which are now required by theInstanceConfigAuditUtilclass.
65-67: Added stub configuration for mock BizConfig.Good setup of the mock
BizConfigwith sensible default values for the configuration parameters that will be used by theInstanceConfigAuditUtil.
69-69: Updated constructor call with new dependencies.The constructor call has been updated to include the new
bizConfigandmeterRegistrydependencies, which aligns with the changes in the implementation class.docs/zh/deployment/distributed-deployment-guide.md (4)
1575-1582: Added new configuration for client audit queue size.The documentation nicely explains the
instance.config.audit.max.sizeparameter, which controls the maximum size of the queue for audit records. This is a good addition that gives users control over memory usage related to auditing.
1583-1590: Added new configuration for instance cache size.The documentation for
instance.cache.max.sizeis clearly explained, detailing that it controls the maximum number of instances that can be cached, with eviction mechanisms when the limit is reached.
1591-1598: Added new configuration for instance configuration cache size.Clear explanation of
instance.config.cache.max.sizeparameter which controls the maximum size of the instance configuration cache, similar to the instance cache configuration.
1600-1604: Added time threshold configuration for audit records.The documentation for
instance.config.audit.time.threshold.minutesexplains how this parameter can optimize the auditing process by reducing unnecessary record updates.apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java (6)
19-19: Added imports for new dependencies.Necessary imports added for
BizConfig,MeterRegistry, and Guava cache metrics for the enhanced auditing and monitoring functionality.Also applies to: 34-36
59-62: Refactored field initializations.The initialization of cache fields has been moved from field declaration to methods for better organization and to allow dynamic configuration.
64-66: Added fields for new dependencies.Added private final fields for
BizConfigandMeterRegistryto support configurable cache sizes and metrics collection.
67-78: Enhanced constructor with configuration and metrics dependencies.The constructor now accepts
BizConfigandMeterRegistryparameters, which are used to initialize the audit queue with a configurable size and set up dynamically configured caches. This enhances the flexibility of the class.
144-146: Used configurable time threshold for audit comparison.The method now uses a threshold from
bizConfiginstead of a hardcoded value, making it more configurable and adaptable to different deployment scenarios.
184-194: Extracted instance cache building to a separate method.Created a dedicated method for building the instance cache with configurable maximum size and optional statistics recording. This is a good separation of concerns and enhances maintainability.
...ice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java (1)
36-37: Unused import should be removed.The
javax.annotation.PostConstructimport is not used in the class. According to previous review comments, there was a discussion about whether to use PostConstruct, and it was decided against it, but the import remains.-import javax.annotation.PostConstruct;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (8)
🔇 Additional comments (6)
apollo-configservice/src/main/java/com/ctrip/framework/apollo/configservice/util/InstanceConfigAuditUtil.java (6)
19-19: Good addition of necessary imports for enhanced functionality.The added imports enable the class to leverage configurable parameters via
BizConfigand monitoring capabilities viaMeterRegistryandGuavaCacheMetrics. These are appropriate for the enhanced auditing and caching requirements.Also applies to: 34-36
59-65: Good refactoring of field declarations.The fields have been properly reorganized to support the new configuration-driven approach. The
auditsfield is now declared here but initialized in the constructor with a configurable size, whileBizConfigandMeterRegistryare correctly added as final fields.
67-78: Constructor properly updated to support configurable parameters.The constructor now correctly accepts
BizConfigandMeterRegistrydependencies and initializes theauditsqueue with a configurable size. The extraction of cache building logic into separate methods follows good separation of concerns principles.
145-145: Good replacement of hardcoded value with configurable parameter.Replacing the hardcoded threshold with a configurable parameter from
bizConfigincreases flexibility and allows for runtime adjustments without code changes.
184-194: Well-structured instance cache initialization.Good implementation of a configurable instance cache with the following improvements:
- Configurable maximum size via
bizConfig.getInstanceCacheMaxSize()- Optional statistics recording based on configuration
- Proper metrics registration when statistics are enabled
The code effectively uses the builder pattern and conditional logic to create a cache tailored to current configuration settings.
196-206: Well-implemented instance config release key cache.The implementation properly configures the cache with:
- Configurable maximum size via
bizConfig.getInstanceConfigCacheMaxSize()- Conditional statistics recording
- Appropriate metrics registration
The consistent pattern between this and the instance cache initialization shows good code organization.
youngzil
deleted the
feature/InstanceConfigAuditUtil-parameter-configurability
branch
2 participants
What's the purpose of this PR
After starting the service and making some requests to pull configurations, access
curl 127.0.0.1:8080/prometheus | grep -E 'instance_cache|instance_config_cache'and you can see the following monitoring indicator dataWhich issue(s) this PR fixes:
Fixes #5358
Brief changelog
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean testto make sure this pull request doesn't break anything.CHANGESlog.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor