Skip to content

[hotfix][Optimizer] Preserve keeper visibility on lookup failure - #4325

Open
czy006 wants to merge 2 commits into
apache:masterfrom
czy006:codex/hotfix-optimizer-keeper-visibility
Open

[hotfix][Optimizer] Preserve keeper visibility on lookup failure#4325
czy006 wants to merge 2 commits into
apache:masterfrom
czy006:codex/hotfix-optimizer-keeper-visibility

Conversation

@czy006

@czy006 czy006 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Brief change log

Make optimizer touch-time visibility explicit and keep resource groups under observation when a container lookup fails transiently, so the keeper can retry on a later cycle.

How was this patch tested?

  • Add keeper test coverage for touch-time visibility and lookup-failure requeueing.
  • Add screenshots for manual tests if appropriate (not applicable: backend-only change).
  • Run TestOptimizerGroupKeeper locally with JDK 11 before creating this pull request.

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • no
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • not applicable

@czy006
czy006 marked this pull request as ready for review August 17, 2026 09:11
@github-actions github-actions Bot added the module:ams-server Ams server module label Aug 17, 2026
…d on container lookup failure

touchTime is written by thrift heartbeat threads and read by the keeper
thread without a shared lock - a plain long field is a JMM data race
(stale reads can expire live optimizers; word tearing on 32-bit JVMs).
Declare it volatile.

OptimizerGroupKeeper.processTask called Containers.get outside the
try/finally whose finally-block re-queues the group via keepInTouch.
An unknown container name threw IllegalArgumentException straight into
AbstractKeeper.run's swallow-all catch, permanently removing the group
from scale-out monitoring until restart. Move the lookup and the cast
inside the try.

Regression test testUnknownContainerKeepsGroupWatchedAndResetsMinParallelism
(red before: min-parallelism stayed 2, group silently dropped).
Fix record: docs/fix-records/2026-08-16-fix-15-keeper-visibility-and-container-lookup.md
@czy006
czy006 force-pushed the codex/hotfix-optimizer-keeper-visibility branch from 9df3593 to 31dc7be Compare August 18, 2026 02:16

@zhoujinsong zhoujinsong 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.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module:ams-server Ams server module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants