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.

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

fix: [Cherry-pick] Prevent ChunkCache use absolute path in All-in-one mode (#30666) #30679

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

congqixia
Copy link
Contributor

Cherry pick from master
pr: #30666
See also #30651

Append operator of std::filesystem::path will replace whole path when the param of "/" operation is an absolute path.

In "All-in-one" mode, this shall cause ChunkCache removing the original vector data file when building chunk cache during/after load procedure.

This PR changes the ChunkCache path generation logic to a separate function in which will check whether the file path is absolute or not. If the file path is absolute, it removes the root path prefix and return concatenated file path.


…io#30666)

See also milvus-io#30651

Append operator of `std::filesystem::path` will replace whole path when
the param of "/" operation is an absolute path.

In "All-in-one" mode, this shall cause ChunkCache removing the original
vector data file when building chunk cache during/after load procedure.

This PR changes the ChunkCache path generation logic to a separate
function in which will check whether the file path is absolute or not.
If the file path is absolute, it removes the root path prefix and return
concatenated file path.

---------

Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: congqixia

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot added the size/M Denotes a PR that changes 30-99 lines. label Feb 20, 2024
@mergify mergify bot added dco-passed DCO check passed. kind/bug Issues or changes related a bug labels Feb 20, 2024
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (8c8db10) 82.13% compared to head (d2a1303) 82.16%.
Report is 5 commits behind head on 2.3.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              2.3   #30679      +/-   ##
==========================================
+ Coverage   82.13%   82.16%   +0.03%     
==========================================
  Files         841      831      -10     
  Lines      121379   121302      -77     
==========================================
- Hits        99691    99669      -22     
+ Misses      18442    18395      -47     
+ Partials     3246     3238       -8     
Files Coverage Δ
internal/core/src/storage/ChunkCache.h 100.00% <ø> (ø)
internal/core/src/storage/ChunkCache.cpp 80.00% <90.90%> (+2.44%) ⬆️

... and 168 files with indirect coverage changes

@mergify mergify bot added the ci-passed label Feb 20, 2024
@yah01
Copy link
Member

yah01 commented Feb 20, 2024

/lgtm

@sre-ci-robot sre-ci-robot merged commit 8734bcc into milvus-io:2.3 Feb 20, 2024
14 checks passed
@congqixia congqixia deleted the cp_30666 branch February 21, 2024 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved ci-passed dco-passed DCO check passed. kind/bug Issues or changes related a bug lgtm size/M Denotes a PR that changes 30-99 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants