Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(cubestore): Initial support for kv Cache #5265

Closed
wants to merge 1 commit into from
Closed

feat(cubestore): Initial support for kv Cache #5265

wants to merge 1 commit into from

Conversation

ovr
Copy link
Member

@ovr ovr commented Sep 8, 2022 •
edited
Loading

Hello!

This PR introduces support for CACHE (key, value) storage in Cubestore + driver for Cube.

Supported commands:

CACHE GET
CACHE SET [NX] [TTL ?number] key value
CACHE REMOVE
CACHE TRUNCATE
CACHE KEYS

refs #5042

Thanks

@ovr ovr force-pushed the cubestore-cache-kv-initial branch 8 times, most recently from abade08 to 11a7fea Compare September 14, 2022 16:14
@ovr ovr force-pushed the cubestore-cache-kv-initial branch 6 times, most recently from 6eba544 to ffbe80f Compare October 10, 2022 14:09
@codecov
Copy link

codecov bot commented Oct 10, 2022 •
edited
Loading

Codecov Report

Base: 40.55% // Head: 40.33% // Decreases project coverage by -0.21% ⚠️

Coverage data is based on head (3213503) compared to base (09ceffb).
Patch coverage: 56.52% of modified lines in pull request are covered.

❗ Current head 3213503 differs from pull request most recent head 72702ff. Consider uploading reports for the commit 72702ff to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #5265      +/-   ##
==========================================
- Coverage   40.55%   40.33%   -0.22%     
==========================================
  Files         144      145       +1     
  Lines       19339    19499     +160     
  Branches     4901     4947      +46     
==========================================
+ Hits         7843     7865      +22     
- Misses      11191    11329     +138     
  Partials      305      305
Flag Coverage Δ
cube-backend 40.33% <56.52%> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...-orchestrator/src/orchestrator/LocalCacheDriver.ts 78.78% <ø> (ø)
...-orchestrator/src/orchestrator/RedisCacheDriver.ts 3.63% <0.00%> (-0.14%) ⬇️
...-query-orchestrator/src/orchestrator/QueryQueue.js 67.64% <33.33%> (-1.56%) ⬇️
...orchestrator/src/orchestrator/QueryOrchestrator.ts 57.63% <50.00%> (ø)
...-query-orchestrator/src/orchestrator/QueryCache.ts 73.35% <52.63%> (-1.65%) ⬇️
packages/cubejs-base-driver/src/index.ts 100.00% <100.00%> (ø)
...y-orchestrator/src/orchestrator/BaseQueueDriver.ts 100.00% <100.00%> (ø)
...y-orchestrator/src/orchestrator/PreAggregations.ts 76.20% <100.00%> (ø)
...bejs-schema-compiler/src/adapter/CubeStoreQuery.ts 3.78% <100.00%> (ø)
...cubejs-schema-compiler/src/adapter/QueryBuilder.js 71.42% <100.00%> (+1.05%) ⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ovr ovr force-pushed the cubestore-cache-kv-initial branch 8 times, most recently from 760fef2 to 5930e87 Compare October 19, 2022 15:36
@ovr ovr force-pushed the cubestore-cache-kv-initial branch 4 times, most recently from e9be1c5 to 8f3bdcd Compare October 21, 2022 19:24
ovr
ovr commented Oct 21, 2022
View reviewed changes
packages/cubejs-query-orchestrator/src/orchestrator/PreAggregations.ts Outdated
@@ -1804,15 +1797,15 @@ export class PreAggregations {

protected tablesUsedRedisKey(tableName) {
// TODO add dataSource?
return `SQL_PRE_AGGREGATIONS_${this.redisPrefix}_TABLES_USED_${tableName}`;
return this.queryCache.getKey('SQL_PRE_AGGREGATIONS_TABLES_USED', tableName);
Copy link
Member Author

Choose a reason for hiding this comment

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

Notice: Key change. I know

@ovr ovr force-pushed the cubestore-cache-kv-initial branch 2 times, most recently from 08e577e to f625c65 Compare October 24, 2022 16:43
@ovr ovr force-pushed the cubestore-cache-kv-initial branch 9 times, most recently from d583fbd to 811f6fc Compare November 3, 2022 08:47
@ovr ovr marked this pull request as ready for review November 3, 2022 08:47
@ovr ovr requested review from a team as code owners November 3, 2022 08:47
@ovr ovr force-pushed the cubestore-cache-kv-initial branch from 811f6fc to 7794b8a Compare November 3, 2022 09:02
waralexrom
waralexrom reviewed Nov 4, 2022
View reviewed changes
rust/cubestore/cubestore/src/metastore/mod.rs Outdated
Comment on lines 2062 to 2067
trace!(
"Row exists in secondary index (with TTL) however missing in {:?} table: {}. Compaction problem?",
self, id
);

continue;
Copy link
Member

Choose a reason for hiding this comment

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

It's data consistency problem, not compaction (because of get_row_ids_by_index filter out expired ids). I think we should rebuild index at this case, or, at least put TODO about it here

waralexrom
waralexrom reviewed Nov 4, 2022
View reviewed changes
rust/cubestore/cubestore/src/metastore/mod.rs
}

async fn cache_set(&self, item: CacheItem, nx: bool) -> Result<bool, CubeError> {
self.write_operation_cache(move |db_ref, batch_pipe| {
Copy link
Member

Choose a reason for hiding this comment

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

Please rename nx to something more understandable

@ovr ovr force-pushed the cubestore-cache-kv-initial branch 2 times, most recently from 3213503 to ecfc0e6 Compare November 8, 2022 14:16
@ovr ovr force-pushed the cubestore-cache-kv-initial branch from ecfc0e6 to d5680cf Compare November 24, 2022 19:35
@ovr ovr force-pushed the cubestore-cache-kv-initial branch from d5680cf to 72702ff Compare November 25, 2022 13:03
@ovr ovr closed this Aug 3, 2023
@ovr ovr deleted the cubestore-cache-kv-initial branch August 3, 2023 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waralexrom waralexrom waralexrom left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ovr @waralexrom