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

fix: Milvus panic when compaction disabled and dropping a collection #34103

Merged
merged 10 commits into from
Jul 11, 2024
Merged

fix: Milvus panic when compaction disabled and dropping a collection #34103

merged 10 commits into from
Jul 11, 2024

Conversation

XuanYang-cn
Copy link
Contributor

See also: #31059

@sre-ci-robot sre-ci-robot requested review from cydrain and sunby June 24, 2024 09:50
@sre-ci-robot sre-ci-robot added the size/M Denotes a PR that changes 30-99 lines. label Jun 24, 2024
@mergify mergify bot added dco-passed DCO check passed. kind/bug Issues or changes related a bug labels Jun 24, 2024
@XuanYang-cn
Copy link
Contributor Author

Fixing panic first since there're so many refactory on compaction. Will change it to dynamic later.

@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 24, 2024

@XuanYang-cn E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 24, 2024

@XuanYang-cn ut workflow job failed, comment rerun ut can trigger the job again.

@czs007 czs007 added the PR | need cherry-pick need cherry pick to other branches label Jun 25, 2024
@XuanYang-cn
Copy link
Contributor Author

rerun ut

@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 25, 2024

@XuanYang-cn E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@XuanYang-cn
Copy link
Contributor Author

rerun ut

XuanYang-cn added a commit to XuanYang-cn/milvus that referenced this pull request Jun 26, 2024
@XuanYang-cn
fix: Milvus panic when compaction disabled and dropping a collection
2e9ab6d 
See also: milvus-io#31059
pr: milvus-io#34103

Signed-off-by: yangxuan <[email protected]>
@XuanYang-cn XuanYang-cn mentioned this pull request Jun 26, 2024
Merged
@XuanYang-cn
Copy link
Contributor Author

rerun ut

@czs007 czs007 removed the PR | need cherry-pick need cherry pick to other branches label Jun 26, 2024
@XuanYang-cn
Copy link
Contributor Author

rerun ut

@XuanYang-cn
fix
609f0ae 
Signed-off-by: yangxuan <[email protected]>
@XuanYang-cn XuanYang-cn force-pushed the fix-painc-by-off-compaction branch from c0dfc77 to 609f0ae Compare June 27, 2024 07:12
@sre-ci-robot sre-ci-robot added size/L Denotes a PR that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Jun 27, 2024
@XuanYang-cn
Copy link
Contributor Author

rerun ut

@sre-ci-robot sre-ci-robot added size/M Denotes a PR that changes 30-99 lines. and removed size/L Denotes a PR that changes 100-499 lines. labels Jul 2, 2024
@XuanYang-cn
Copy link
Contributor Author

rerun ut

@mergify Mergify
Copy link
Contributor

mergify bot commented Jul 2, 2024

@XuanYang-cn E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@XuanYang-cn
Copy link
Contributor Author

rerun ut

@sre-ci-robot sre-ci-robot added size/L Denotes a PR that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Jul 8, 2024
@mergify mergify bot added the ci-passed label Jul 9, 2024
@codecov Codecov
Copy link

codecov bot commented Jul 9, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 92.10526% with 3 lines in your changes missing coverage. Please review.

Project coverage is 79.81%. Comparing base (3333160) to head (09dbfd6).
Report is 21 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #34103       +/-   ##
===========================================
+ Coverage   69.77%   79.81%   +10.04%     
===========================================
  Files        1127     1127               
  Lines      139419   139417        -2     
===========================================
+ Hits        97279   111281    +14002     
+ Misses      37855    23843    -14012     
- Partials     4285     4293        +8
Files Coverage Δ
internal/datacoord/compaction_trigger_v2.go 72.25% <100.00%> (+0.16%) ⬆️
internal/datacoord/server.go 72.89% <100.00%> (+0.13%) ⬆️
internal/datacoord/compaction_trigger.go 86.00% <88.88%> (-0.38%) ⬇️
internal/datacoord/services.go 84.30% <50.00%> (-0.02%) ⬇️

... and 264 files with indirect coverage changes

@tedxu
Copy link
Contributor

tedxu commented Jul 11, 2024

/lgtm

sre-ci-robot pushed a commit that referenced this pull request Jul 11, 2024
@XuanYang-cn
fix: Milvus panic when compaction disabled and dropping a collection (#…
7e2a9d6 
…34206)

See also: #31059
pr: #34103

---------

Signed-off-by: yangxuan <[email protected]>
@congqixia
Copy link
Contributor

/approve

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: congqixia, XuanYang-cn

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 merged commit e0b39d8 into milvus-io:master Jul 11, 2024
12 checks passed
@XuanYang-cn XuanYang-cn deleted the fix-painc-by-off-compaction branch July 11, 2024 06:48
chyezh pushed a commit to chyezh/milvus that referenced this pull request Jul 15, 2024
@XuanYang-cn @chyezh
fix: Milvus panic when compaction disabled and dropping a collection (m…
bf6fabe 
…ilvus-io#34103)

See also: milvus-io#31059

---------

Signed-off-by: yangxuan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cydrain cydrain Awaiting requested review from cydrain

@sunby sunby Awaiting requested review from sunby

Assignees

@tedxu tedxu

Labels
approved ci-passed dco-passed DCO check passed. kind/bug Issues or changes related a bug lgtm size/L Denotes a PR that changes 100-499 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@XuanYang-cn @tedxu @congqixia @sre-ci-robot @czs007