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

better lightning-loader configuration #3519

Closed
lance6716 opened this issue Nov 18, 2021 · 5 comments · Fixed by #4034
Closed

better lightning-loader configuration #3519

lance6716 opened this issue Nov 18, 2021 · 5 comments · Fixed by #4034
Assignees
Labels
area/dm Issues or PRs related to DM. subject/new-feature Denotes an issue or pull request adding a new feature.

Comments

@lance6716
Copy link
Contributor

Is your feature request related to a problem?

currently DM's config for lightning loader is not well-designed

Describe the feature you'd like

Regarding the new backend configuration item, I hope it could be optimized.

tidb:
  backend: "tidb"
  1. location
    Since this configuration is related to target, I think it can be placed in
target-database:
  host: "127.0.0.1"
  port: 4400
  user: "root"
  password: ""
  security:
    ssl-ca: "dir-placeholer/task-ca.pem"
    ssl-cert: "dir-placeholer/dm.pem"
    ssl-key: "dir-placeholer/dm.key"
  import-mode: "${value}"  # If multiple downstreams (N->N) are to be added in the future, this approach can also continue to be used
  1. the value of the configuration item is not easy to understand, for historical reasons Lightning called tidb and local two modes. We can improve it in dm so that new users can understand it better
  • tidb/local could be replaced with logical/physical or sql/sst?,looks better to understand

Describe alternatives you've considered

No response

Teachability, Documentation, Adoption, Migration Strategy

No response

@lance6716 lance6716 added the subject/new-feature Denotes an issue or pull request adding a new feature. label Nov 18, 2021
@lance6716
Copy link
Contributor Author

An alternative for feature 1 is we put import-mode under loader/full-phase configuration, that's more reasonable when we think of the functionality, but we should find a global "loader/full-phase configuration" to put this item.

@sunzhaoyang
Copy link

I put it under target-database because, if a task allows multiple targets in the future, it may be possible to use different import-mode for different targets

@lance6716
Copy link
Contributor Author

cc @glorv

@lance6716 lance6716 added the area/dm Issues or PRs related to DM. label Nov 18, 2021
@glorv glorv self-assigned this Nov 19, 2021
@glorv
Copy link
Contributor

glorv commented Nov 23, 2021

I put it under target-database because, if a task allows multiple targets in the future, it may be possible to use different import-mode for different targets

Since the load backend (loader or lightning) is only effect the load phase, I think it's more reasonable to put it under loader. BTW, this config should be a invisible config.
A brief roadmap of this config:
Sprint 6: Support lightning(default) and loader, loader should be only used in some special cases(when lightning has compatible or performance issues).
Sprint 7: Permanently abandon loader and add local (lightning local backend) to load phase.
Sprint 8: DM can automatically choose between tidb or local backend in load phase.

@lance6716
Copy link
Contributor Author

from the functionality, this field should be put under loader section.

But we should also make sure this field is related to downstream and loader section is referenced by upstream instances.

@glorv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dm Issues or PRs related to DM. subject/new-feature Denotes an issue or pull request adding a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants