Skip to content

Fix InstanceTraffic.combine overwriting last_ping with an older minute - #14146

Merged
wu-sheng merged 1 commit into
apache:masterfrom
Dev-next-gen:fix/instance-traffic-last-ping
Oct 16, 2026
Merged

wu-sheng merged 1 commit into
apache:masterfrom
Dev-next-gen:fix/instance-traffic-last-ping

Conversation

@Dev-next-gen

Copy link
Copy Markdown
Contributor

Fix a service instance dropping out of the time-ranged instance list after a late update

  • Add a unit test to verify that the fix works.
  • Explain briefly why the bug exists and how to fix it.

InstanceTraffic.combine copies the last_ping of the incoming record, so a record carrying an older minute moves last_ping backwards. The heartbeat writes the current minute (ManagementServiceHandler uses the server clock), while a segment writes the minute its span started (RPCAnalysisListener). When the segment of a request that started at 14:21 arrives after a heartbeat at 14:25, the stored last_ping becomes 14:21, and listInstances with a duration starting at 14:22 or later, which filters on last_ping >= start, leaves the instance out until a newer record lands.

This is the same cause #14122 fixed in EndpointTraffic.combine; this change keeps the larger last_ping the same way.

InstanceTrafficTest combines a stored 14:25 with a 14:21 record and then a 14:30 one. Before the change it fails with expected: <202003281425> but was: <202003281421>; after it passes, and EndpointTraffic*Test still pass.

Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5.5.

A segment carries the minute its span started, so the segment of a
request that began minutes earlier can update an instance after its
heartbeat wrote the current minute. combine copied that older last_ping,
and the instance dropped out of the instance list queried with a time
range starting after it. Keep the larger last_ping, as EndpointTraffic
does since apache#14122.
@wu-sheng wu-sheng added this to the 11.1.0 milestone Oct 15, 2026
@wu-sheng wu-sheng added backend OAP backend related. chore Chores about the project, like code cleaning up, typos, upgrading dependencies, etc. labels Oct 15, 2026
@wu-sheng

Copy link
Copy Markdown
Member

Note, this is a small safeguard against out-of-order timestamps. Keeping the maximum fits the existing instance interval: earliest timeBucket, latest last_ping.

@Dev-next-gen

Dev-next-gen commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor Author

The one red check, Istio ALS (mx-mesh, BanyanDB, 1.25.0, 28), fails before any test runs: the e2e setup step helm -n istio-system install skywalking ... ends with Error: INSTALLATION FAILED: failed post-install: 1 error occurred: * timed out waiting for the condition (job). The other 35 Istio ALS variants of this run, which deploy the same OAP image, passed.

The same Helm post-install timeout happened on master at 217e2e6 in Istio ALS (k8s-mesh, BanyanDB, 1.23.0, 28) (job), so I read it as a flaky install step rather than something this change causes. A re-run of that job should clear it.

Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5.5.

@wu-sheng
wu-sheng merged commit e775a04 into apache:master Oct 16, 2026
479 of 481 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend OAP backend related. chore Chores about the project, like code cleaning up, typos, upgrading dependencies, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants