Skip to content

Conversation

@ovr
Copy link
Member

@ovr ovr commented Mar 3, 2023

Hello!

refs #5042

Thanks

@ovr ovr requested a review from a team as a code owner March 3, 2023 15:38
@ovr ovr force-pushed the cachestore-separate-loop-queue branch from ee0f7d5 to 82780a4 Compare July 1, 2025 14:05
@ovr ovr force-pushed the cachestore-separate-loop-queue branch from 82780a4 to 90c1c83 Compare July 1, 2025 14:29
@ovr ovr requested a review from Copilot July 2, 2025 10:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds a reusable read/write loop abstraction and applies it to both the core store and the cache queue.

  • Introduces RocksStoreRWLoop to encapsulate a blocking Tokio channel and processing loop.
  • Refactors RocksStore to use write_operation_impl/read_operation_impl with loop names in spans.
  • Updates RocksCacheStore to use a separate rw_loop_queue_cf and adds write_operation_queue/read_operation_queue.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
rust/cubestore/cubestore/src/metastore/rocks_store.rs Added RocksStoreRWLoop, refactored read/write ops to use it.
rust/cubestore/cubestore/src/cachestore/cache_rocksstore.rs Added queue-specific RW loop, replaced direct store calls.
Comments suppressed due to low confidence (1)

rust/cubestore/cubestore/src/metastore/rocks_store.rs:808

  • [nitpick] The RocksStoreRWLoop struct is public but lacks doc comments. Consider adding /// comments to explain its purpose and usage.
#[derive(Debug, Clone)]

@ovr ovr force-pushed the cachestore-separate-loop-queue branch from cd461cf to 9e66023 Compare July 2, 2025 12:29
@ovr ovr merged commit f961f97 into master Jul 2, 2025
51 of 52 checks passed
@ovr ovr deleted the cachestore-separate-loop-queue branch July 2, 2025 13:03
Frank-TXS pushed a commit to Helge-TXS/cube that referenced this pull request Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants