Skip to content

api: support temporary configuration#3082

Merged
ti-srebot merged 12 commits intotikv:masterfrom
Yisaer:temp_config
Oct 21, 2020
Merged

api: support temporary configuration#3082
ti-srebot merged 12 commits intotikv:masterfrom
Yisaer:temp_config

Conversation

@Yisaer
Copy link
Contributor

@Yisaer Yisaer commented Oct 19, 2020

What problem does this PR solve?

What is changed and how it works?

  1. Provide a temporary API to support temporary configuration for some configurations.

  2. Make /store/{id}/limit and /stores/limit support ttl param.

Note that this API is only supported for lightning and BR as a temporary way. Don't call this API.

As the temporary configuration only is saved in the memory, a recommended way to use API is calling it with short interval (once a minute) and long ttl (10 minutes).

Release note

Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
@Yisaer
Copy link
Contributor Author

Yisaer commented Oct 19, 2020

@3pointer @glorv @overvenus PTAL

@Yisaer Yisaer added the component/api HTTP API. label Oct 19, 2020
@Yisaer Yisaer requested review from disksing and rleungx October 19, 2020 05:10
@Yisaer Yisaer added the needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. label Oct 19, 2020
Signed-off-by: Song Gao <disxiaofei@163.com>
// allows to access them safely.
type PersistOptions struct {
// configuration -> ttl value
ttl map[string]*cache.TTLString
Copy link
Member

Choose a reason for hiding this comment

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

What if the PD restart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we didn't persist it, so we need lightning and BR to call this API with a short interval (like once a minute)

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 19, 2020
Copy link
Contributor

@lhy1024 lhy1024 left a comment

Choose a reason for hiding this comment

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

the rest LGTM

c.Assert(s.svr.GetPersistOptions().GetStoreLimit(1).RemovePeer, Equals, float64(999))
c.Assert(s.svr.GetPersistOptions().GetMaxMergeRegionSize(), Equals, uint64(999))
c.Assert(s.svr.GetPersistOptions().GetMaxMergeRegionKeys(), Equals, uint64(999))
time.Sleep(10 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

It will delay the test time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

}

// @Tags config
// @Summary Update temporary configuration. Note that this is a temporary api and only support for lightning and BR. Don't call this API and it will be removed in release-5.0.
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to add a warning in the return result?

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 think the Warning in @description is enough.

"store-limit-add-peer": 999,
"max-merge-region-size": 999,
"max-merge-region-keys": 999,
"store-limit-remove-peer": 999,
Copy link
Member

Choose a reason for hiding this comment

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

BR needs enable-location-replacement too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
@Yisaer Yisaer removed the status/DNM label Oct 19, 2020
@Yisaer Yisaer requested review from lhy1024 and rleungx October 20, 2020 02:49
apiRouter.HandleFunc("/config/cluster-version", confHandler.SetClusterVersion).Methods("POST")
apiRouter.HandleFunc("/config/replication-mode", confHandler.GetReplicationMode).Methods("GET")
apiRouter.HandleFunc("/config/replication-mode", confHandler.SetReplicationMode).Methods("POST")
apiRouter.HandleFunc("/config/ttl", confHandler.SetTTLConfig).Methods("POST")
Copy link
Contributor

Choose a reason for hiding this comment

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

why not consist with store api, use /config?ttlSecond=xx?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

}

var ttl int
if ttlSec := r.URL.Query().Get("ttlSecond"); ttlSec != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

update the swager comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
@nolouch
Copy link
Contributor

nolouch commented Oct 21, 2020

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 21, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Oct 21, 2020
@Yisaer
Copy link
Contributor Author

Yisaer commented Oct 21, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Oct 21, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 47e83f7 into tikv:master Oct 21, 2020
ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Oct 21, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #3088

ti-srebot added a commit that referenced this pull request Oct 22, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/api HTTP API. needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants