Skip to content

Commit

Permalink
Merge branch 'master' into audit_middleware
Browse files Browse the repository at this point in the history
  • Loading branch information
ti-chi-bot authored Feb 11, 2022
2 parents 1b170fe + ea69785 commit 436892f
Show file tree
Hide file tree
Showing 31 changed files with 319 additions and 211 deletions.
18 changes: 17 additions & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,26 @@ You can use it with query parameters: https://github.com/tikv/pd/compare/master.
-->

### What problem does this PR solve?
<!--
Please create an issue first to describe the problem.
There MUST be one line starting with "Issue Number: " and
linking the relevant issues via the "close" or "ref".
For more info, check https://github.com/tikv/pd/blob/master/CONTRIBUTING.md#linking-issues.
<!-- Add the issue link with a summary if it exists. -->
-->
Issue Number: Close #xxx

### What is changed and how it works?
<!--
You could use "commit message" code block to add more description to the final commit message.
For more info, check https://github.com/tikv/pd/blob/master/CONTRIBUTING.md#format-of-the-commit-message.
-->

```commit-message
```

### Check List

Expand Down
70 changes: 45 additions & 25 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,12 @@ If you are planning something big, for example, relates to multiple components o

This is a rough outline of what a contributor's workflow looks like:

- Create an issue about the problem that you are going to solve if there isn't existed.
- Create an issue about the problem that you are going to solve if there isn't existed (see below for linking issue).
- Create a topic branch from where you want to base your work. This is usually master.
- Make commits of logical units and add test case if the change fixes a bug or adds new functionality.
- Run tests and make sure all the tests are passed.
- Make sure your commit messages are in the proper format (see below).
- Push your changes to a topic branch in your fork of the repository.
- Submit a pull request.
- Submit a pull request and make sure proper final commit message (see below for message format).
- Your PR must receive LGTMs from two reviewers.

More specifics on the development workflow are in [development workflow](./docs/development-workflow.md).
Expand All @@ -39,42 +38,63 @@ The coding style suggested by the Golang community is used in PD. See the [style

Please follow this style to make PD easy to review, maintain and develop.

### Format of the Commit Message
### Linking issues

Code repositories in TiKV community require **ALL** the pull requests referring to its corresponding issues. In the pull request body, there **MUST** be one line starting with `Issue Number: ` and linking the relevant issues via the [keyword](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), for example:

We follow a rough convention for commit messages that is designed to answer two
questions: what changed and why. The subject line should feature the what and
the body of the commit should describe the why.
If the pull request resolves the relevant issues, and you want GitHub to close these issues automatically after it merged into the default branch, you can use the syntax (`KEYWORD #ISSUE-NUMBER`) like this:

```
server/schedule: add comment for variable declaration
Issue Number: close #123
```

Improve documentation.
If the pull request links an issue but does not close it, you can use the keyword `ref` like this:

```
Issue Number: ref #456
```

Multiple issues should use full syntax for each issue and separate by a comma, like:

```
Issue Number: close #123, ref #456
```

The format can be described more formally as follows:
For pull requests trying to close issues in a different repository, contributors need to first create an issue in the same repository and use this issue to track.

If the pull request body does not provide the required content, the bot will add the `do-not-merge/needs-linked-issue` label to the pull request to prevent it from being merged.

### Format of the Commit Message

The bot we use will extract the pull request title as the one-line subject and messages inside the `commit-message` code block as commit message body. For example, a pull request with title `pkg: what's changed in this one package` and body containing:

```commit-message
any multiple line commit messages that go into
the final commit message body

* fix something 1
* fix something 2
```

will get a final commit message:

```
<subsystem>: <what changed>
<BLANK LINE>
<why this change was made>
<BLANK LINE>
<close/fix/ref/resolve> #<issue number>
<BLANK LINE>
<footer>
pkg: what's changed in this one package (#12345)
any multiple line commit messages that go into
the final commit message body
* fix something 1
* fix something 2
```

The first line is the subject and should be no longer than 70 characters, the
second line is always blank, and other lines should be wrapped at 80 characters.
This allows the message to be easier to read on GitHub as well as in various
git tools.
The first line is the subject (the pull request title) and should be no longer than 70 characters, the second line is always blank, and other lines should be wrapped at 80 characters. This allows the message to be easier to read on GitHub as well as in various git tools.

If the change affects more than one subsystem, you can use comma to separate them like `server, pd-cilent:`.
If the change affects more than one subsystem, you can use comma to separate them like `server, pd-client:`.

If the change affects many subsystems, you can use ```*``` instead, like ```*:```.

For the why part, if no specific reason for the change,
you can use one of some generic reasons like "Improve documentation.",
"Improve performance.", "Improve robustness.", "Improve test coverage."
The body of the commit message should describe why the change was made and at a high level, how the code works.

### Signing off the Commit

Expand Down
2 changes: 1 addition & 1 deletion client/option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (s *testClientSuite) TestDynamicOptionChange(c *C) {
expectBool := true
o.setEnableTSOFollowerProxy(expectBool)
// Check the value changing notification.
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
<-o.enableTSOFollowerProxyCh
return true
})
Expand Down
6 changes: 3 additions & 3 deletions pkg/mock/mockhbstream/mockhbstream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ func (s *testHeartbeatStreamSuite) TestActivity(c *C) {

// Active stream is stream1.
hbs.BindStream(1, stream1)
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
hbs.SendMsg(region, proto.Clone(msg).(*pdpb.RegionHeartbeatResponse))
return stream1.Recv() != nil && stream2.Recv() == nil
})
// Rebind to stream2.
hbs.BindStream(1, stream2)
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
hbs.SendMsg(region, proto.Clone(msg).(*pdpb.RegionHeartbeatResponse))
return stream1.Recv() == nil && stream2.Recv() != nil
})
Expand All @@ -83,7 +83,7 @@ func (s *testHeartbeatStreamSuite) TestActivity(c *C) {
c.Assert(res.GetHeader().GetError(), NotNil)
// Switch back to 1 again.
hbs.BindStream(1, stream1)
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
hbs.SendMsg(region, proto.Clone(msg).(*pdpb.RegionHeartbeatResponse))
return stream1.Recv() != nil && stream2.Recv() == nil
})
Expand Down
4 changes: 2 additions & 2 deletions pkg/testutil/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const (

// CheckFunc is a condition checker that passed to WaitUntil. Its implementation
// may call c.Fatal() to abort the test, or c.Log() to add more information.
type CheckFunc func(c *check.C) bool
type CheckFunc func() bool

// WaitOp represents available options when execute WaitUntil
type WaitOp struct {
Expand Down Expand Up @@ -63,7 +63,7 @@ func WaitUntil(c *check.C, f CheckFunc, opts ...WaitOption) {
opt(option)
}
for i := 0; i < option.retryTimes; i++ {
if f(c) {
if f() {
return
}
time.Sleep(option.sleepInterval)
Expand Down
1 change: 1 addition & 0 deletions server/api/pprof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func (s *ProfSuite) TearDownSuite(c *C) {
func (s *ProfSuite) TestGetZip(c *C) {
rsp, err := testDialClient.Get(s.urlPrefix + "/pprof/zip?" + "seconds=5s")
c.Assert(err, IsNil)
defer rsp.Body.Close()
body, err := ioutil.ReadAll(rsp.Body)
c.Assert(err, IsNil)
c.Assert(body, NotNil)
Expand Down
2 changes: 1 addition & 1 deletion server/api/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func mustNewCluster(c *C, num int, opts ...func(cfg *config.Config)) ([]*config.
}

func mustWaitLeader(c *C, svrs []*server.Server) {
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
var leader *pdpb.Member
for _, svr := range svrs {
l := svr.GetLeader()
Expand Down
3 changes: 3 additions & 0 deletions server/api/stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ func (s *testStatsSuite) TestRegionStats(c *C) {
}
res, err := testDialClient.Get(statsURL)
c.Assert(err, IsNil)
defer res.Body.Close()
stats := &statistics.RegionStats{}
err = apiutil.ReadJSON(res.Body, stats)
c.Assert(err, IsNil)
Expand All @@ -150,6 +151,7 @@ func (s *testStatsSuite) TestRegionStats(c *C) {
args := fmt.Sprintf("?start_key=%s&end_key=%s", url.QueryEscape("\x01\x02"), url.QueryEscape("xyz\x00\x00"))
res, err = testDialClient.Get(statsURL + args)
c.Assert(err, IsNil)
defer res.Body.Close()
stats = &statistics.RegionStats{}
err = apiutil.ReadJSON(res.Body, stats)
c.Assert(err, IsNil)
Expand All @@ -171,6 +173,7 @@ func (s *testStatsSuite) TestRegionStats(c *C) {
args = fmt.Sprintf("?start_key=%s&end_key=%s", url.QueryEscape("a"), url.QueryEscape("x"))
res, err = testDialClient.Get(statsURL + args)
c.Assert(err, IsNil)
defer res.Body.Close()
stats = &statistics.RegionStats{}
err = apiutil.ReadJSON(res.Body, stats)
c.Assert(err, IsNil)
Expand Down
2 changes: 1 addition & 1 deletion server/api/tso_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (s *testTsoSuite) TearDownSuite(c *C) {
}

func (s *testTsoSuite) TestTransferAllocator(c *C) {
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
s.svr.GetTSOAllocatorManager().ClusterDCLocationChecker()
_, err := s.svr.GetTSOAllocatorManager().GetAllocator("dc-1")
return err == nil
Expand Down
3 changes: 3 additions & 0 deletions server/api/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func (s *testUtilSuite) TestJsonRespondErrorOk(c *C) {
c.Assert(input["zone"], Equals, output["zone"])
c.Assert(input["host"], Equals, output["host"])
result := response.Result()
defer result.Body.Close()
c.Assert(result.StatusCode, Equals, 200)
}

Expand All @@ -55,6 +56,7 @@ func (s *testUtilSuite) TestJsonRespondErrorBadInput(c *C) {
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, "json: cannot unmarshal object into Go value of type []string")
result := response.Result()
defer result.Body.Close()
c.Assert(result.StatusCode, Equals, 400)

{
Expand All @@ -64,6 +66,7 @@ func (s *testUtilSuite) TestJsonRespondErrorBadInput(c *C) {
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, "unexpected end of JSON input")
result := response.Result()
defer result.Body.Close()
c.Assert(result.StatusCode, Equals, 400)
}
}
12 changes: 6 additions & 6 deletions server/cluster/coordinator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ func BenchmarkPatrolRegion(b *testing.B) {
}

func waitOperator(c *C, co *coordinator, regionID uint64) {
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
return co.opController.GetOperator(regionID) != nil
})
}
Expand Down Expand Up @@ -1206,7 +1206,7 @@ func (s *testScheduleControllerSuite) TestInterval(c *C) {

func waitAddLearner(c *C, stream mockhbstream.HeartbeatStream, region *core.RegionInfo, storeID uint64) *core.RegionInfo {
var res *pdpb.RegionHeartbeatResponse
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
if res = stream.Recv(); res != nil {
return res.GetRegionId() == region.GetID() &&
res.GetChangePeer().GetChangeType() == eraftpb.ConfChangeType_AddLearnerNode &&
Expand All @@ -1222,7 +1222,7 @@ func waitAddLearner(c *C, stream mockhbstream.HeartbeatStream, region *core.Regi

func waitPromoteLearner(c *C, stream mockhbstream.HeartbeatStream, region *core.RegionInfo, storeID uint64) *core.RegionInfo {
var res *pdpb.RegionHeartbeatResponse
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
if res = stream.Recv(); res != nil {
return res.GetRegionId() == region.GetID() &&
res.GetChangePeer().GetChangeType() == eraftpb.ConfChangeType_AddNode &&
Expand All @@ -1239,7 +1239,7 @@ func waitPromoteLearner(c *C, stream mockhbstream.HeartbeatStream, region *core.

func waitRemovePeer(c *C, stream mockhbstream.HeartbeatStream, region *core.RegionInfo, storeID uint64) *core.RegionInfo {
var res *pdpb.RegionHeartbeatResponse
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
if res = stream.Recv(); res != nil {
return res.GetRegionId() == region.GetID() &&
res.GetChangePeer().GetChangeType() == eraftpb.ConfChangeType_RemoveNode &&
Expand All @@ -1255,7 +1255,7 @@ func waitRemovePeer(c *C, stream mockhbstream.HeartbeatStream, region *core.Regi

func waitTransferLeader(c *C, stream mockhbstream.HeartbeatStream, region *core.RegionInfo, storeID uint64) *core.RegionInfo {
var res *pdpb.RegionHeartbeatResponse
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
if res = stream.Recv(); res != nil {
if res.GetRegionId() == region.GetID() {
for _, peer := range append(res.GetTransferLeader().GetPeers(), res.GetTransferLeader().GetPeer()) {
Expand All @@ -1273,7 +1273,7 @@ func waitTransferLeader(c *C, stream mockhbstream.HeartbeatStream, region *core.
}

func waitNoResponse(c *C, stream mockhbstream.HeartbeatStream) {
testutil.WaitUntil(c, func(c *C) bool {
testutil.WaitUntil(c, func() bool {
res := stream.Recv()
return res == nil
})
Expand Down
Loading

0 comments on commit 436892f

Please sign in to comment.