server: support resigning ddl owner, use http method ddl/owner/resign #7649
Conversation
…into evict_owner_by_http
server/http_handler.go
Outdated
| defer s.Close() | ||
| } | ||
|
|
||
| dom := domain.GetDomain(s.(sessionctx.Context)) |
There was a problem hiding this comment.
We can use session.GetDomain().
There was a problem hiding this comment.
What is the difference? Use session.GetDomain() will look likes:
dom, err := session.GetDomain(s.GetStore())
if err != nil {
return errors.Trace(err)
}
There was a problem hiding this comment.
The difference is that we do not need to create a session.
docs/tidb_http_api.md
Outdated
| 1. Resign the ddl owner, let tidb start a new ddl owner election. | ||
|
|
||
| ```shell | ||
| curl http://{TiDBIP}:10080/ddl/owner/resign |
server/http_handler.go
Outdated
| @@ -334,6 +353,11 @@ type ddlHistoryJobHandler struct { | |||
| *tikvHandlerTool | |||
| } | |||
|
|
|||
| // ddlResignOwnerHandler is the handler for resigning ddl owner. | |||
| type ddlResignOwnerHandler struct { | |||
| *tikvHandlerTool | |||
There was a problem hiding this comment.
This is not operated on TiKV.
How about we implement the method on ddlResignOwnerHandler directly?
|
@coocood @lamxTyler PTAL |
|
Can we just use |
|
@coocood use |
|
@coocood fine, if using |
…into evict_owner_by_http
|
@coocood @lamxTyler Done, PTAL |
owner/manager.go
Outdated
|
|
||
| func (m *ownerManager) retireOwner() { | ||
| m.SetOwner(false) | ||
| atomic.StorePointer(&m.elec, nil) |
There was a problem hiding this comment.
Could we only use the elec to determine if it is the owner? Then we do not need to maintain the owner anymore.
There was a problem hiding this comment.
There are some test cases use SetOwner(false) and just use elec to determin if it is the owner will not easy to mock in tests.
There was a problem hiding this comment.
Can we change SetOwner(isOwner bool) to SetOwner(elec unsafe.Pointer)?
In the mockManager, we can use any not nil pointer to represent elec.
| return errors.Trace(err) | ||
| } | ||
|
|
||
| log.Warnf("%s Resign ddl owner success!", m.logPrefix) |
There was a problem hiding this comment.
No, this is not the work of ResignOwner.
|
LGTM |
coocood
added
status/LGT2
owner/manager.go
Outdated
| @@ -45,12 +46,14 @@ type Manager interface { | |||
| ID() string | |||
| // IsOwner returns whether the ownerManager is the owner. | |||
| IsOwner() bool | |||
| // SetOwner sets whether the ownerManager is the owner. | |||
| SetOwner(isOwner bool) | |||
| // RetireOwner make the manager to be a not owner. It's exported for testing | |||
owner/mock.go
Outdated
| atomic.StoreInt32(&m.owner, 1) | ||
| } | ||
|
|
||
| // setOwner implements Manager.RetireOwner interface. |
| } | ||
|
|
||
| ownerMgr := dom.DDL().OwnerManager() | ||
| err = ownerMgr.ResignOwner(context.Background()) |
There was a problem hiding this comment.
Do we need to add a timeout?
There was a problem hiding this comment.
@winkyao Please answer this question.
…into evict_owner_by_http
|
@zimulala PTAL |
|
/run-all-tests |
winkyao
added
status/all tests passed
release-note
What problem does this PR solve?
Support resigning DDL owner, let tidb start a new DDL owner election.
What is changed and how it works?
Add a new HTTP method
ddl/owner/resignand this method will callconcurrency.Election.Resign.concurrency.Election.Resignwill delete the leader key/tidb/ddl/fg/owner/on the etcd. And the owner of TiDB will exitwatchOwnerfunction, and TiDB cluster will start a new election byelec.Campaign.Check List
Tests
use ansible to start a new cluster with two tidb instances. Run
curl http://{TiDBIP}:10080/ddl/owner/resignin non-owner will responseThis node is not a ddl owner, can't be resigned., and the owner will return"success!".And use
select tidb_is_ddl_owner, we can find out the owner of the tidb is transferred to the other instance.Code changes
Side effects
Related changes