Skip to content

fix(metrics): improve probe sample shutdown behavior#1107

Merged
Alanxtl merged 2 commits into
apache:mainfrom
Snow-kal:fix/metrics-probe-dependency
May 18, 2026
Merged

fix(metrics): improve probe sample shutdown behavior#1107
Alanxtl merged 2 commits into
apache:mainfrom
Snow-kal:fix/metrics-probe-dependency

Conversation

@Snow-kal

Copy link
Copy Markdown
Contributor

更改内容:
metrics/probe/go-server/cmd/main.go
新增 shutdownDrainSeconds = 6
收到 SIGINT/SIGTERM 后,先把 probe 状态标记为 NotReady/NotStarted
然后等待 6 秒,让 Kubernetes 的 readinessProbe 有机会探测到 NotReady

metrics/probe/deploy/server-deployment.yml
新增 terminationGracePeriodSeconds: 15
新增容器资源配置:requests: cpu: 100m, memory: 128Mi
limits: cpu: 500m, memory: 512Mi

issue: apache/dubbo-go#3204 metrics

问题
在按照 readme 多次测试的时候 docker k8s 可能会出现 还没等重新探测到 NotReady 就退出的情况

现在已经更改,结果与 readme 一样
image

Copilot AI review requested due to automatic review settings May 16, 2026 13:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adjusts the probe sample's shutdown timing and Kubernetes deployment resource configuration to make readiness state changes observable during pod termination.

Changes:

  • Increased the post-shutdown sleep in the Go server to exceed the readiness probe period.
  • Added terminationGracePeriodSeconds to the server deployment.
  • Added CPU/memory requests and limits for the server container.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
metrics/probe/go-server/cmd/main.go Adds shutdownDrainSeconds constant and uses it for the final sleep so NotReady state can be observed.
metrics/probe/deploy/server-deployment.yml Adds termination grace period and resource requests/limits to the server container.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +22 to +28
resources:
requests:
cpu: 100m
memory: 128Mi
limits:
cpu: 500m
memory: 512Mi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这个是用来限制什么的

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

主要是来避免容器没有资源边界
requests 限制最低资源需求
imits 表示容器运行时最多可使用的资源上限

当然如果觉得不是很稳妥的话 可以把这个 server-deployment.yml 回到原来版本 没啥影响

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

感觉 requests 可以删了,应该用不到这么多内存吧?具体需要多少我不是很了解

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OK 已经删除 恢复原样了

@Alanxtl Alanxtl merged commit 3e5d977 into apache:main May 18, 2026
2 checks passed
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.

3 participants