Skip to content

Clean up temp files generated in s3_plain_rewritable moving file transaction.#88006

Merged
tuanpach merged 2 commits intoClickHouse:masterfrom
tuanpach:fix-issue-87656
Oct 8, 2025
Merged

Clean up temp files generated in s3_plain_rewritable moving file transaction.#88006
tuanpach merged 2 commits intoClickHouse:masterfrom
tuanpach:fix-issue-87656

Conversation

@tuanpach
Copy link
Copy Markdown
Member

@tuanpach tuanpach commented Oct 2, 2025

Changelog category (leave one):

  • CI Fix or Improvement (changelog entry is not required)

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

Clean up temp files generated in MetadataStorageFromPlainObjectStorageMoveFileOperation when loading table metadata in DatabaseOnDisk.

When moving files, the logic generated some temporary files. In DatabaseOnDisk, we should clean them during the table metadata loading.

Re-enable remote_database_disk in integration tests with is_local_server_asan_build.

Closes #87656

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

…eMoveFileOperation when loading table metadata in DatabaseOnDisk
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Oct 2, 2025

Workflow [PR], commit [c97f745]

Summary:

@clickhouse-gh clickhouse-gh bot added the pr-ci label Oct 2, 2025
@jkartseva jkartseva self-assigned this Oct 2, 2025
Copy link
Copy Markdown
Member

@jkartseva jkartseva left a comment

Choose a reason for hiding this comment

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

LGTM

{
{
auto tmp_path_from = path_to.string() + ".move_from." + getRandomASCIIString(16);
auto tmp_path_from = path_to.string() + "." + getRandomASCIIString(16) + ".tmp_move_from";
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.

Do we care about leftover files with names in the old format, which will not be removed because they don't end with ".tmp_move_from" (or ".tmp_move_to")?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The default database disk is local. This case has only occurred in the CI jobs, so I think we don't need to handle the old file name format.

@tuanpach tuanpach added this pull request to the merge queue Oct 8, 2025
Merged via the queue into ClickHouse:master with commit 79640bf Oct 8, 2025
123 checks passed
@tuanpach tuanpach deleted the fix-issue-87656 branch October 8, 2025 07:34
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-ci pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integration tests can be flaky due to database disk

3 participants