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: use correct delete checkpoint to prevent premature data cleanup #40366

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: use correct delete checkpoint to prevent premature data cleanup #40366

wants to merge 1 commit into from
+63 −23

Conversation

weiliu1031
Copy link
Contributor

@weiliu1031 weiliu1031 commented Mar 5, 2025 •
edited
Loading

issue: #40292
related to #39552

  • Fix incorrect delete checkpoint usage in SyncDistribution
  • Change checkpoint parameter from action.GetCheckpoint() to action.GetDeleteCP() in SyncTargetVersion call
  • This resolves the issue where delete buffer data was being cleaned prematurely due to wrong checkpoint reference

@sre-ci-robot sre-ci-robot requested review from aoiasd and congqixia March 5, 2025 06:33
@sre-ci-robot sre-ci-robot added the size/S Denotes a PR that changes 10-29 lines. label Mar 5, 2025
@weiliu1031 weiliu1031 force-pushed the fix_premature_clean_delete branch from ed27dfd to c93698a Compare March 5, 2025 06:33
@mergify mergify bot added dco-passed DCO check passed. kind/bug Issues or changes related a bug labels Mar 5, 2025
@weiliu1031 weiliu1031 mentioned this pull request Mar 5, 2025
Open
1 task
@mergify Mergify
Copy link
Contributor

mergify bot commented Mar 5, 2025

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

@weiliu1031
Copy link
Contributor Author

/run-cpu-e2e

@mergify Mergify
Copy link
Contributor

mergify bot commented Mar 5, 2025

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

@codecov Codecov
Copy link

codecov bot commented Mar 5, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.97%. Comparing base (837ac29) to head (543485b).
Report is 21 commits behind head on master.

❌ Your project check has failed because the head coverage (69.97%) is below the target coverage (77.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (837ac29) and HEAD (543485b). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (837ac29) HEAD (543485b)
3 1
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #40366       +/-   ##
===========================================
- Coverage   80.53%   69.97%   -10.57%     
===========================================
  Files        1470      309     -1161     
  Lines      206664    27749   -178915     
===========================================
- Hits       166442    19418   -147024     
+ Misses      34177     8331    -25846     
+ Partials     6045        0     -6045
Components Coverage Δ
Client ∅ <ø> (∅)
Core 69.97% <ø> (+<0.01%) ⬆️
Go ∅ <ø> (∅)

see 1165 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mergify mergify bot added the ci-passed label Mar 5, 2025
@weiliu1031
fix: use correct delete checkpoint to prevent premature data cleanup
543485b 
- Fix incorrect delete checkpoint usage in SyncDistribution
- Change checkpoint parameter from action.GetCheckpoint() to action.GetDeleteCP() in SyncTargetVersion call
- This resolves the issue where delete buffer data was being cleaned prematurely due to wrong checkpoint reference

Signed-off-by: Wei Liu <[email protected]>
@weiliu1031 weiliu1031 force-pushed the fix_premature_clean_delete branch from c93698a to 543485b Compare March 6, 2025 03:51
@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: weiliu1031
To complete the pull request process, please assign jiaoew1991 after the PR has been reviewed.
You can assign the PR to them by writing /assign @jiaoew1991 in a comment when ready.

The full list of commands accepted by this bot can be found 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 size/M Denotes a PR that changes 30-99 lines. and removed size/S Denotes a PR that changes 10-29 lines. labels Mar 6, 2025
@mergify mergify bot removed the ci-passed label Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aoiasd aoiasd Awaiting requested review from aoiasd

@congqixia congqixia Awaiting requested review from congqixia

Assignees
No one assigned
Labels
component/querynode dco-passed DCO check passed. kind/bug Issues or changes related a bug size/M Denotes a PR that changes 30-99 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@weiliu1031 @sre-ci-robot