[da-vinci][cdc][server][test] DVRT CDC improvements and RocksDB optimizations #2458
Conversation
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
version-specific flag Changed the logic in VeniceChangelogConsumerDaVinciRecordTransformerImpl.buildVeniceConfig() to set PUSH_STATUS_STORE_ENABLED based on changelogClientConfig.isStateful() instead of !isVersionSpecificClient. This aligns the push status store configuration with whether the client is stateful or stateless. Also added unit tests to verify the behavior and renamed statefulVeniceChangelogConsumer to veniceChangelogConsumer in tests for clarity. Co-Authored-By: Claude Opus 4.5 <[email protected]>
configuration Add DaVinciConfig option to disable RocksDB block cache for individual stores. Beneficial for sequential access patterns where data is read once (e.g., sequential scans, changelog consumers). Prevents cache pollution of the shared block cache used by other DaVinci stores and reduces memory pressure. CDC clients automatically enable this optimization since they only read each key once from disk. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
…roperties Make buildVeniceConfig() method final in VeniceChangelogConsumerDaVinciRecordTransformerImpl to prevent overriding in tests. Refactor changelog consumer tests to use shared consumer properties initialized in setUp() instead of recreating them in each test method. Add ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES configuration set to 0 to disable block cache in tests. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
…nning Previously, seekToBeginningOfPush delegated to subscribe() which passed null for checkpointInfo, causing the ingestion task to resume from persisted offsets on disk instead of starting from the beginning of the push. This change implements proper seekToBeginningOfPush by: - Adding seekToBeginningOfPush flag to DaVinciSeekCheckpointInfo - Converting the flag to PubSubSymbolicPosition.EARLIEST in StoreBackend - Adding seekToBeginningOfPush() method to AvroGenericDaVinciClient - Updating VeniceChangelogConsumerDaVinciRecordTransformerImpl to use initializeAndSubscribe pattern (like seekToCheckpoint/seekToTimestamps) - Making subscribe()/subscribeAll() delegate to seekToBeginningOfPush() to ensure stateless CDC consumers start from the beginning Added comprehensive tests: - 9 unit tests for DaVinci client and changelog consumer - 1 integration test for DVRT-based stateless changelog consumer Co-Authored-By: Claude Sonnet 4.5 (1M context) <[email protected]>
Set ROCKSDB_BLOCK_CACHE_SIZE_IN_BYTES to 0 in consumer properties during setUp to prevent 'Cannot setup rocksdb instance with block-cache size 16.0 GiB. System memory: 15.6 GiB' error in test environments with limited memory. Changes: - TestVersionSpecificChangelogConsumer: Initialize consumerProperties in setUp with block cache disabled - TestActiveActiveVersionSwapMessage: Initialize consumerProperties in setUp with block cache disabled - Both tests now follow the pattern established in TestChangelogConsumer All integration tests passing. Co-Authored-By: Claude Sonnet 4.5 (1M context) <[email protected]>
Add comprehensive test for RocksDBStoragePartition disableBlockCache feature to improve diff coverage from 41% to 50% (exceeds 45% threshold). The test validates: - StoragePartitionConfig getter/setter for disableBlockCache - RocksDB partition creation with cache disabled vs enabled - Read/write operations work correctly in both configurations - Default value is false This ensures the per-store RocksDB block cache disable feature (added in 243b26e) has proper test coverage and prevents regressions. Co-Authored-By: Claude Sonnet 4.5 (1M context) <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem Statement
seekToBeginningOfPush()incorrectly resumed from persisted offsets instead of starting from the beginningSolution
seekToBeginningOfPush()to properly start from beginning of push usingPubSubSymbolicPosition.EARLIESTisStateful()flagCode changes
Concurrency-Specific Checks
Both reviewer and PR author to verify
synchronized,RWLock) are used where needed.ConcurrentHashMap,CopyOnWriteArrayList).How was this PR tested?
Does this PR introduce any user-facing or breaking changes?