MyScaleDB icon indicating copy to clipboard operation
MyScaleDB copied to clipboard

Fix merge bugs due to path error

Open ucasfl opened this issue 1 year ago • 2 comments

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

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

Fix merge fail with vector index.

Documentation entry for user-facing changes

  • [ ] Documentation is written (mandatory for new features)

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

ucasfl avatar Nov 08 '24 07:11 ucasfl

Hi ucasfl,

Thank you for the PR and for taking the time to address this issue!

To help us understand this fix better, could you please provide a description of the bug, including the specific scenario or conditions that trigger it? Additionally, if you have any test cases to verify the fix, that would be very helpful for ensuring everything works as expected and for preventing similar issues in the future.

Thanks again for your contribution!

Aed-p avatar Nov 11 '24 03:11 Aed-p

@Aed-p Use inverted_row_ids_map_file_path as example. https://github.com/myscale/MyScaleDB/blob/3bd6368796c670d56b01d83e2ba58066efb646dc/src/Storages/MergeTree/MergeTask.cpp#L397-L398

It include the full path of the disk, and writeFile only need to provede the file name: https://github.com/myscale/MyScaleDB/blob/3bd6368796c670d56b01d83e2ba58066efb646dc/src/Storages/MergeTree/MergeTask.cpp#L639-L640 https://github.com/myscale/MyScaleDB/blob/3bd6368796c670d56b01d83e2ba58066efb646dc/src/Storages/MergeTree/DataPartStorageOnDiskFull.cpp#L111-L121

So we should use fileName to get the file name when calling writeFile like other places, otherwise, the path pass into Disk become full_disk_path/full_disk_path/filename, leading to file does not exist error due to directory does not exist. https://github.com/myscale/MyScaleDB/blob/3bd6368796c670d56b01d83e2ba58066efb646dc/src/Storages/MergeTree/MergeTask.cpp#L634 https://github.com/myscale/MyScaleDB/blob/3bd6368796c670d56b01d83e2ba58066efb646dc/src/Storages/MergeTree/MergeTask.cpp#L298

ucasfl avatar Nov 11 '24 04:11 ucasfl