tests/server: reduce test time for heartbeat msg cannot be triggered

[HuSharp]

What problem does this PR solve?

Issue Number: Ref #7969

What is changed and how does it work?

Performance

can reduce test time from 5s+1min(wait for keepalive) to 5s

Why change?

Previous test want to mock client send region heartbeat to server, and then server will return msg to client.

But Previous test only can be triggered by keepalive

pd/pkg/schedule/hbstream/heartbeat_streams.go

Line 154 in f51f913

case <-keepAliveTicker.C:

So I changed region id to 0 which will send error msg

pd/server/grpc_service.go

Lines 1279 to 1284 in 5c58ddd

	if region.GetID() == 0 {
	regionHeartbeatCounter.WithLabelValues(storeAddress, storeLabel, "report", "invalid-region").Inc()
	msg := fmt.Sprintf("invalid request region, %v", request)
	s.hbStreams.SendErr(pdpb.ErrorType_UNKNOWN, msg, request.GetLeader())
	continue
	}

and then client will get msg by

pd/pkg/schedule/hbstream/heartbeat_streams.go

Line 127 in f51f913

case msg := <-s.msgCh:

Check List

Tests

• Unit test

Release note

None.

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

[codecov]

Codecov Report

Merging #7998 (18e96a6) into master (67461dd) will increase coverage by 0.03%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7998      +/-   ##
==========================================
+ Coverage   77.27%   77.30%   +0.03%     
==========================================
  Files         468      468              
  Lines       60890    60890              
==========================================
+ Hits        47055    47074      +19     
+ Misses      10291    10288       -3     
+ Partials     3544     3528      -16     

Flag	Coverage Δ
unittests	77.30% <ø> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[rleungx]

Will it skip some processes of handle region heartbeat?

[HuSharp]

yes, but this regionReq wants to ensure the client can get the server's response.

• only call SendScheduleCommand will send right msg by oc.hbStreams.SendMsg(region, cmd)
• DispatchFromHeartBeat will not trigger SendScheduleCommand

Otherwise, we can only get the msg when the hbStream's keepalive is triggered, which for this test is no different than sending an error msg.

And the concurrent region heartbeat tests are handled (the purpose of this test) in the below code.

[rleungx]

I don't remember what is the purpose of this test. Does it aim to test the concurrent problem or race problem?

[HuSharp]

Yes, Seems like testing(including race) for concurrent running HandleRegionHeartbeat

[HuSharp]

I think core logic for this test is here
https://github.com/tikv/pd/pull/7998/files#diff-4f23e8da76548a94e6779db8719386a8eabb85ad56deca5e6271a8e162652baaR789-R818

[rleungx]

If the heartbeat process exits earlier, will it still take effect?

[HuSharp]

I think this part is just like a ping which wants to ensure the connection between the client and server.
So I think the msg of the response is unimportant :)

And I have added some breakpoints to ensure concurrent region heartbeat has been handled.

[HuSharp]

@rleungx @lhy1024 friendly ping :)

[HuSharp]

@rleungx @lhy1024 friendly ping :)