venice
venice copied to clipboard
[server][dvc] Improve ST logging and add test coverage for endPosition subscription
[server][dvc] Improve ST logging and add test coverage for endPosition subscription
Enhances server-side ST logging by including store version status and more descriptive replica-role details. Refactors store name and version parsing to avoid repeated extraction and centralizes store-type and version-role logic for clarity and reuse. Adds tests to verify the contract for endPosition subscription in PubSubConsumerAdapter.
AI Generated Summary Of Changes
Refactoring and code reuse:
- The constructor of
AbstractPartitionStateModelnow parses and storesstoreNameandversionNumberfrom theresourceNameonce, making these values directly accessible throughout the class and reducing repeated parsing. Related getter methods (getStoreName,getVersionNumber) are added. [1] [2] [3] - Logic for determining the store type (
VeniceUserStoreType) and the version role (e.g., CURRENT, BACKUP, FUTURE) is centralized in new or refactored helper methods (determineStoreType,getStoreType,getStoreVersionRole). This improves code maintainability and makes these utilities available for logging and other purposes.
Logging improvements:
- Log messages for state transitions now include detailed replica type descriptions (e.g., "Hybrid store current version"), making logs more informative for debugging and operations. [1] [2] [3]
Simplification and consistency:
- All uses of store name and version number in
LeaderFollowerPartitionStateModelare updated to use the new getter methods, reducing code duplication and risk of parsing errors. [1] [2] [3] [4] - The store type determination logic is moved from inline code to a dedicated method, and the previous implementation is removed for clarity. [1] [2]
Test improvements:
- The test setup in
AbstractVenicePartitionStateModelTestis updated to use the new utility methods and to improve mock consistency, making tests clearer and more robust. [1] [2] [3] [4] [5]
Code changes
- [ ] Added new code behind a config. If so list the config names and their default values in the PR description.
- [ ] Introduced new log lines.
- [ ] Confirmed if logs need to be rate limited to avoid excessive logging.
Concurrency-Specific Checks
Both reviewer and PR author to verify
- [ ] Code has no race conditions or thread safety issues.
- [ ] Proper synchronization mechanisms (e.g.,
synchronized,RWLock) are used where needed. - [ ] No blocking calls inside critical sections that could lead to deadlocks or performance degradation.
- [ ] Verified thread-safe collections are used (e.g.,
ConcurrentHashMap,CopyOnWriteArrayList). - [ ] Validated proper exception handling in multi-threaded code to avoid silent thread termination.
How was this PR tested?
- [ ] New unit tests added.
- [ ] New integration tests added.
- [ ] Modified or extended existing tests.
- [ ] Verified backward compatibility (if applicable).
Does this PR introduce any user-facing or breaking changes?
- [ ] No. You can skip the rest of this section.
- [ ] Yes. Clearly explain the behavior change and its impact.