Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support ttl check and update ttl #17

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nnothing1
Copy link

@CLAassistant
Copy link

CLAassistant commented Dec 2, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nnothing1
To complete the pull request process, please assign baiyutang after the PR has been reviewed.
You can assign the PR to them by writing /assign @baiyutang in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@GuangmingLuo
Copy link
Member

@baiyutang 麻烦看看

return c.consulClient.Agent().ServiceDeregister(svcID)
}

// startTTLHeartbeat start a goroutine to periodically update TTL.
func (c *consulRegistry) startTTLHeartbeat(ttl time.Duration) error {
if ttl <= 1*time.Second {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个参数校验可以放在更前边,放在注册之前去校验 Agent().ServiceRegister(svcInfo);

// wait for health check passing
time.Sleep(time.Second * 6)

list, _, err := consulClient.Health().Service(testSvcName, "", true, nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里去获取服务实例列表,建议多等待几个 TTL 的周期,或者连续去校验几个 TTL 周期,证明更新是有效的。

ctx, cancel := context.WithCancel(context.Background())
c.cancelUpdateTTL = cancel
go func() {
if err := c.consulClient.Agent().UpdateTTL(c.opts.check.CheckID, "online", api.HealthPassing); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

另外还应该评估下 Deregister 时 是否有必要主动停止这里的 更新,如果 调用 Deregister 但是距离真正服务停止还有一段时间,是否会有时间空隙,然后客户端会获取到这个准备下线的实例,去请求却又请求不到 造成失败。这种可以评估一下。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can not use TTL checks
4 participants