feat: support windows , run in multiple networks and support heartbeat #321
Conversation
| [APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: timandy The full list of commands accepted by this bot can be found here. Needs approval from an approver in each of these files: Approvers can indicate their approval by writing |
| /assign @lyyyuna |
| test case failed caused by server write log not flushed to disk. The ci in my fork repo is successed. |
| nonLocalIPV4 = ipNet.IP.String() | ||
| } | ||
| } | ||
| func getRealHost(port int) (host string, err error) { |
There was a problem hiding this comment.
host and err are not in use.
| fmt.Fprintf(f, strings.TrimPrefix(profileAddr, "http://")) | ||
| host, err := getRealHost(port) | ||
| if err != nil { |
There was a problem hiding this comment.
this err isn't caught. we'd better print it to let outside knows.
There was a problem hiding this comment.
The error already printed when the app startup. And I add logs when the heartbeat failed.
| Hi @timandy, Thanks for your contributions. This seems a little big changes and we need more verifications especially for the new adapters on windows & k8s env. Also, i am thinking whether it is necessary or not for the heartbeats enhancement, since now |
… such as run in docker
…ave been inactive for a long time
@CarlJi Thanks for your review. Our service is deployed in the container. After several restarts, a lot of invalid clients will be generated. Although the goc server can filter invalid clients, these operations need network probing, which affects performance. |
Codecov Report
Additional details and impacted files
@@ Coverage Diff @@ ## master #321 +/- ## ========================================== - Coverage 70.76% 67.69% -3.07% ========================================== Files 34 35 +1 Lines 2035 2130 +95 ========================================== + Hits 1440 1442 +2 - Misses 482 577 +95 + Partials 113 111 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| [APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: timandy The full list of commands accepted by this bot can be found here. Needs approval from an approver in each of these files: Approvers can indicate their approval by writing |
4 participants
Hi, there.
I made some features, hope can help you.