Skip to content

Improve drop cache query#36639

Merged
kssenii merged 4 commits intoClickHouse:masterfrom
kssenii:better-drop-cache
Apr 26, 2022
Merged

Improve drop cache query#36639
kssenii merged 4 commits intoClickHouse:masterfrom
kssenii:better-drop-cache

Conversation

@kssenii
Copy link
Copy Markdown
Member

@kssenii kssenii commented Apr 25, 2022

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Improve SYSTEM DROP FILESYSTEM CACHE query: <path> option and FORCE option.

@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Apr 25, 2022
@tavplubix tavplubix self-assigned this Apr 25, 2022
Comment on lines +349 to +358
case Type::DROP_FILESYSTEM_CACHE:
{
ParserLiteral path_parser;
ASTPtr ast;
if (path_parser.parse(pos, ast, expected))
res->filesystem_cache_path = ast->as<ASTLiteral>()->value.safeGet<String>();
if (ParserKeyword{"FORCE"}.ignore(pos, expected))
res->force_removal = true;
break;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't forget to update ASTSystemQuery::formatImpl as well

Comment on lines 455 to 458
assertNotDetached();

std::lock_guard cache_lock(cache->mutex);
std::lock_guard segment_lock(mutex);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It it ok to check it with unlocked mutexes?

@kssenii kssenii merged commit 81db081 into ClickHouse:master Apr 26, 2022
@tavplubix
Copy link
Copy Markdown
Member

@kssenii, your forgot to take a look at failed tests before merging, you will find Stress test (debug, actions) failure interesting

@kssenii
Copy link
Copy Markdown
Member Author

kssenii commented Apr 26, 2022

@kssenii, your forgot to take a look at failed tests before merging, you will find Stress test (debug, actions) failure interesting

yes :( but needed this PR to be merged asap as it will help with something in cloud. Stress test fix: #36660

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants