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

owner: ignore the invalid admin jobs #600

Merged
merged 11 commits into from
Jun 3, 2020
Merged

Conversation

zier-one
Copy link
Contributor

@zier-one zier-one commented May 26, 2020

What problem does this PR solve?

fix #541 #542.(3)
ignore and warn the invalid admin jobs.

if users remove the changefeed, the owner will delete the changefeed info immediately and set 24h TTL to the changefeed status.

the CDC CLI will report a warning to tall users the changefeed has been deleted.

./cdc cli changefeed remove --changefeed-id 04dac05c-484d-49e2-8163-1cca0d7f2

./cdc cli changefeed query --changefeed-id 04dac05c-484d-49e2-8163-1cca0d7f2fb5
[2020/06/01 09:07:24.984 +00:00] [WARN] [client_changefeed.go:149] ["this changefeed has been deleted, the residual meta data will be completely deleted within 24 hours."]
{
  "info": null,
  "status": {
    "resolved-ts": 417071719969980417,
    "checkpoint-ts": 417071719707836417,
    "admin-job-type": 3
  },
  "count": 0,
  "task-status": []
}                                                             

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test

Release note

@zier-one zier-one requested a review from overvenus May 26, 2020 08:57
@zier-one zier-one added the status/ptal Could you please take a look? label May 27, 2020
Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

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

Could you add a test?

cdc/owner.go Outdated Show resolved Hide resolved
cdc/owner.go Outdated Show resolved Hide resolved
@zier-one zier-one added WIP and removed status/ptal Could you please take a look? labels May 27, 2020
@zier-one zier-one added status/ptal Could you please take a look? and removed WIP labels Jun 1, 2020
@@ -143,6 +145,9 @@ func newQueryChangefeedCommand() *cobra.Command {
taskStatus = append(taskStatus, captureTaskStatus{CaptureID: captureID, TaskStatus: status})
}
meta := &cfMeta{Info: info, Status: status, Count: count, TaskStatus: taskStatus}
if info == nil {
log.Warn("this changefeed has been deleted, the residual meta data will be completely deleted within 24 hours.")
Copy link
Member

Choose a reason for hiding this comment

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

How do we test it if it is get deleted after 24 hours?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i will add a unit test for it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PTAL again

cdc/kv/etcd_test.go Outdated Show resolved Hide resolved
Copy link
Member

@overvenus overvenus 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

cdc/kv/etcd_test.go Outdated Show resolved Hide resolved
cdc/kv/etcd_test.go Outdated Show resolved Hide resolved
@zier-one
Copy link
Contributor Author

zier-one commented Jun 2, 2020

/run-integration-tests

@overvenus overvenus added LGT1 and removed status/ptal Could you please take a look? labels Jun 2, 2020
@zier-one
Copy link
Contributor Author

zier-one commented Jun 2, 2020

/run-integration-tests

@zier-one
Copy link
Contributor Author

zier-one commented Jun 2, 2020

/run-integration-tests

@codecov-commenter
Copy link

Codecov Report

Merging #600 into master will increase coverage by 0.1002%.
The diff coverage is 25.9259%.

@@               Coverage Diff                @@
##             master       #600        +/-   ##
================================================
+ Coverage   33.3333%   33.4335%   +0.1002%     
================================================
  Files            74         74                
  Lines          7626       7651        +25     
================================================
+ Hits           2542       2558        +16     
- Misses         4902       4905         +3     
- Partials        182        188         +6     

@zier-one zier-one merged commit 018f65e into pingcap:master Jun 3, 2020
@zier-one zier-one deleted the fix_541 branch June 8, 2020 03:30
5kbpers pushed a commit to 5kbpers/ticdc that referenced this pull request Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

abnormal behavior when resuming a stopped changefeed
3 participants