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

sync pd and tikv configmap in controller #1330

Merged
merged 2 commits into from
Dec 13, 2019
Merged

Conversation

aylei
Copy link
Contributor

@aylei aylei commented Dec 12, 2019

Signed-off-by: Aylei rayingecho@gmail.com

What problem does this PR solve?

close #1263
close #1264

Check List

Tests

  • Unit test
  • E2E test

Code changes

  • Has Go code change

Does this PR introduce a user-facing change?:

pd and tikv configs are now managed in TidbCluster resource.

@aylei
Copy link
Contributor Author

aylei commented Dec 12, 2019

/run-e2e-test

Signed-off-by: Aylei <rayingecho@gmail.com>
@aylei
Copy link
Contributor Author

aylei commented Dec 13, 2019

/run-e2e-test

@@ -250,7 +246,7 @@ type PDScheduleConfig struct {
// MaxStoreDownTime is the max duration after which
// a store will be considered to be down if it hasn't reported heartbeats.
// +optional
MaxStoreDownTime Duration `toml:"max-store-down-time,omitempty" json:"max-store-down-time,omitempty"`
MaxStoreDownTime string `toml:"max-store-down-time,omitempty" json:"max-store-down-time,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BurntSushi/toml do not respect MarshalText, use string instead

Copy link
Contributor

Choose a reason for hiding this comment

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

BurntSushi/toml seems not maintained anymore...

we can use https://github.com/pelletier/go-toml and implement toml.MarshalTOML() (like MarshalJSON())

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@aylei aylei Dec 13, 2019

Choose a reason for hiding this comment

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

Yes, that's exactly what I've thought.

Actually I've tried go-toml( and naoina/toml), it turns out that go-toml does not support interface{} (necessary for schemaless configuration, e.g. Pump). Also, PD and TiDB uses BurntSushi/toml so there is little verifying work, seems like that BurntSushi/toml is not an ideal but the only choice

@aylei
Copy link
Contributor Author

aylei commented Dec 13, 2019

/run-e2e-test

Copy link
Contributor

@cofyc cofyc left a comment

Choose a reason for hiding this comment

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

LGTM

@aylei aylei merged commit 5ead47e into pingcap:master Dec 13, 2019
yahonda pushed a commit that referenced this pull request Dec 27, 2021
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.

sync tikv configmap and start script in controller sync pd configmap and start script in controller
3 participants