Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

Some SwitchMode improvements #287

Merged
merged 2 commits into from
Apr 13, 2020
Merged

Some SwitchMode improvements #287

merged 2 commits into from
Apr 13, 2020

Conversation

kennytm
Copy link
Collaborator

@kennytm kennytm commented Mar 23, 2020

What problem does this PR solve?

Fix #134 and AskTUG-33070.

What is changed and how it works?

  1. During switch mode, if we encounter a store that returns Unimplemented (mainly TiFlash nowadays), simply ignore the error.
  2. Added ./tidb-lightning-ctl --fetch-mode subcommand to print the mode.

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Side effects

Related changes

  • Need to update the documentation
  • Need to be included in the release note
    • Document the new --fetch-mode flag.

@kennytm kennytm added Should Update Docs Should update docs after this PR is merged. Remove this label once the docs are updated status/PTAL This PR is ready for review. Add this label back after committing new changes priority/normal type/feature New feature 3.0-release-note Should include in release note for next 3.0 release. Remove after release. 3.1-release-note Should include in release note for next 3.1 release. Remove after release. 4.0-release-note Should include in release note for next 4.0 release. Remove after release. labels Mar 23, 2020
@IANTHEREAL
Copy link
Collaborator

take a time to fix ci 😄

@IANTHEREAL
Copy link
Collaborator

This pr just ignores these error logs temporarily. If ideal, the tikv client should be able to handle the incomplete implementation of these interfaces, right?

@IANTHEREAL
Copy link
Collaborator

LGTM

@kennytm
Copy link
Collaborator Author

kennytm commented Mar 28, 2020

This pr just ignores these error logs temporarily. If ideal, the tikv client should be able to handle the incomplete implementation of these interfaces, right?

No that's not intended to be temporary. The reason is that failing to switch mode won't cause import to fail (just slower), so ignoring the "Unimplemented" error permanently is fine.

Yes TiFlash should implement the SwitchMode API (by doing nothing at least), which will make the error go truly away.

@kennytm
Copy link
Collaborator Author

kennytm commented Mar 28, 2020

take a time to fix ci 😄

[2020-03-23T21:43:10.004Z] tests/run.sh: line 30: 17815 Aborted                 (core dumped) bin/tikv-server -C "$TEST_DIR/tikv-config.toml"

We're always enabling TLS and thus hit tikv/tikv#7209 😭. Either we block on tikv/tikv#7251 or disable the TLS temporarily.


/run-all-tests tikv=release-3.1 pd=release-3.1 tidb=release-3.1

@kennytm
Copy link
Collaborator Author

kennytm commented Apr 6, 2020

/run-all-tests

@kennytm kennytm added status/LGT1 One reviewer already commented LGTM (LGTM1) and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Apr 7, 2020
@kennytm
Copy link
Collaborator Author

kennytm commented Apr 7, 2020

PTAL @3pointer

@kennytm kennytm added status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) and removed status/LGT1 One reviewer already commented LGTM (LGTM1) labels Apr 13, 2020
@kennytm kennytm merged commit 56d0f7d into master Apr 13, 2020
@kennytm kennytm deleted the kennytm/mode-stuff branch April 13, 2020 10:13
@kennytm kennytm removed the Should Update Docs Should update docs after this PR is merged. Remove this label once the docs are updated label Jun 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3.0-release-note Should include in release note for next 3.0 release. Remove after release. 3.1-release-note Should include in release note for next 3.1 release. Remove after release. 4.0-release-note Should include in release note for next 4.0 release. Remove after release. status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) type/feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

view cluster no-op or importer mode
3 participants