Skip to content
This repository was archived by the owner on Jul 24, 2024. It is now read-only.

test: fix lighting unit test TestBatchSplitRegionByRanges and a panic issue#1086

Merged
ti-chi-bot merged 7 commits into
pingcap:masterfrom
glorv:fix-local-test
May 24, 2021
Merged

test: fix lighting unit test TestBatchSplitRegionByRanges and a panic issue#1086
ti-chi-bot merged 7 commits into
pingcap:masterfrom
glorv:fix-local-test

Conversation

@glorv
Copy link
Copy Markdown
Collaborator

@glorv glorv commented May 6, 2021

What problem does this PR solve?

  • Make the function call count atomic since the BatchSplitRegions method is now called in multiple routines.
  • Do not try to flush and unsafe ingest closed engine.

What is changed and how it works?

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • No release note

@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 6, 2021

release-4.0 branch is fix in #957 at 915afe9

@glorv glorv requested a review from 3pointer May 6, 2021 11:33
@ti-chi-bot ti-chi-bot added size/M and removed size/S labels May 6, 2021
@glorv glorv changed the title test: fix lighting unit test TestBatchSplitRegionByRanges test: fix some lighting unstable tests May 6, 2021
@glorv glorv changed the title test: fix some lighting unstable tests test: fix lighting unit test TestBatchSplitRegionByRanges and a panic issue May 6, 2021
Copy link
Copy Markdown
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

rest LGTM

Comment thread pkg/lightning/backend/local/local.go Outdated
Comment on lines +1135 to +1138
if engineFile.closed.Load() {
return nil
}
engineFile.closed.Store(true)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if engineFile.closed.Load() {
return nil
}
engineFile.closed.Store(true)
if engineFile.closed.Swap(true) {
return nil
}

@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 7, 2021

/run-all-tests

@overvenus
Copy link
Copy Markdown
Member

/lgtm

@ti-chi-bot ti-chi-bot added the status/LGT1 LGTM1 label May 8, 2021
@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 10, 2021

/run-all-tests

@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 10, 2021

/run-all-tests

3 similar comments
@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 10, 2021

/run-all-tests

@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 10, 2021

/run-all-tests

@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 10, 2021

/run-all-tests

@glorv glorv force-pushed the fix-local-test branch from 76620c7 to b31b141 Compare May 10, 2021 11:45
Copy link
Copy Markdown
Collaborator

@Little-Wallace Little-Wallace left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot
Copy link
Copy Markdown
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • Little-Wallace
  • overvenus

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Details

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 LGTM2 and removed status/LGT1 LGTM1 labels May 14, 2021
@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 14, 2021

/merge

@ti-chi-bot
Copy link
Copy Markdown
Member

This pull request has been accepted and is ready to merge.

DetailsCommit hash: ef38b22

@glorv
Copy link
Copy Markdown
Collaborator Author

glorv commented May 24, 2021

/merge

@ti-chi-bot
Copy link
Copy Markdown
Member

This pull request has been accepted and is ready to merge.

DetailsCommit hash: 6131cc0

@ti-chi-bot ti-chi-bot merged commit d067310 into pingcap:master May 24, 2021
ti-chi-bot pushed a commit to ti-chi-bot/br that referenced this pull request May 24, 2021
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Copy Markdown
Member

In response to a cherrypick label: new pull request created: #1138.

3pointer pushed a commit to 3pointer/br that referenced this pull request Jun 4, 2021
3pointer pushed a commit to 3pointer/br that referenced this pull request Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants