Skip to content

feat(admin,loadbalancer): add readiness admission and explicit resume for business instances (#8042) - #8130

Merged
Aias00 merged 2 commits into
apache:masterfrom
konlue:feat/8042-readiness-admission
Oct 30, 2026
Merged

Aias00 merged 2 commits into
apache:masterfrom
konlue:feat/8042-readiness-admission

Conversation

@konlue

@konlue konlue commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Second delivery slice of #8042, after the draining boundary in #8101. A registered business instance can now declare that it is alive but not ready (NOT_READY), and can explicitly resume (READY). Gateways stop selecting it until the resume; heartbeats still cannot restore readiness or cancel a drain.

State model

Event status draining notReady selectable
REGISTER (fresh) unchanged cleared cleared yes (legacy default)
HEARTBEAT refreshed unchanged unchanged per markers
DRAINING unchanged true unchanged no
NOT_READY unchanged unchanged true no
READY unchanged cleared cleared yes
OFFLINE offline path n/a n/a no

READY is the only client-owned "admit me" transition and clears both markers. There is no operator-driven drain yet; if one is added, READY must not silently cancel it (needs source/epoch precedence, deferred). NOT_READY never clears a drain; both markers may coexist and exclusion is an OR. A fresh registration resets both so legacy clients keep today's behavior.

Change

  • New EventType.NOT_READY / READY; ShenyuClientRegisterRepository.notReady/ready and the HTTP client implementation (same URI registration endpoint as DRAINING).
  • Admin merges markers into the discovery upstream's free-form props (notReady, and READY removes both notReady and draining in one read-modify-write); the status column is never touched, so heartbeat-driven status flips cannot restore readiness.
  • ShenyuClientRegisterService / fallback / subscriber wiring; doDiscoveryLocal now clears both markers on a fresh registration.
  • Gateway UpstreamProps.isReady() (default true), Upstream.ready, and UpstreamCacheManager admission: draining || !ready is removed from both health pools and cannot be probe-restored; a later sync without the marker re-admits through the normal paths. Divide and WebSocket handlers copy the flag.
  • No new transport, no DB migration; the marker survives status churn by design.

Compatibility / rollout

Appending enum constants is source-compatible but not wire-compatible in the failing direction. Jackson rejects unknown enum values, so an old Admin returns HTTP 400 for the new events (same contract as DRAINING).

Rollout order: Admin → gateways → clients.
Old clients never send the new events (default ready); old gateways ignore the unknown props key.

Tests / verification

  • Admin: Marker merge/removal, idempotency, unknown handler/url, heartbeat never writes props, subscriber routing, fallback delegation, doNotReady / doReady wiring.
  • Gateway: UpstreamProps default/parse, cache exclusion and re-admission (including all-not-ready and draining+not-ready), Divide/WebSocket propagation.
  • Client: HTTP repository sends NOT_READY / READY with the URI registration path.
  • Full suite: shenyu-admin 2507 tests; changed modules all green; checkstyle 0.
  • Mutation check: Removing the isReady predicate fails the new not-ready cache tests.

Deferred

Part of #8042.

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

Reviewed 1518ad4. The normal NOT_READY/HEARTBEAT/DRAINING/READY flow preserves admission markers and delivers them through the production discovery sync and Divide cache paths. One blocking transition gap remains: a fresh REGISTER for an existing healthy endpoint can bypass the new readiness reset entirely and leave a restarted default-ready/legacy client excluded (inline).

Validation: 202 focused tests passed across Admin, load-balancing, plugin props/Divide/WebSocket handlers, the HTTP registration client and websocket sync handlers. An independent production-path probe passed 16 assertions using the real URI subscriber, Divide registration service, H2/MyBatis upstream persistence, Spring transaction interception, DataChangedEventDispatcher, Admin websocket emitter, gateway websocket parser, common discovery subscriber and Divide handler/cache. Only the websocket network-send boundary and unrelated collaborators were mocked. It confirmed marker preservation through heartbeats and a gateway health round, OR exclusion of the two markers, explicit READY re-admission, and the accepted fresh-REGISTER failure with an explicit READY positive control. Used ./mvnw with Corretto 21.0.13 and target 17; Checkstyle passed, RAT/JaCoCo were skipped.

No full reactor/E2E, live websocket connection or multi-Admin/native-database concurrency execution is claimed. The documented deferred boot-session fencing, gRPC picker consistency, propagation acknowledgements and operator-drain precedence are not findings against this delivery slice.

@konlue

konlue commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Fixed — the reset was indeed owned by the handle-change path.

Change:

  • doRegisterURI no longer gates doDiscoveryLocal on a non-null buildHandle result.
  • The selector handle update stays conditional (a duplicate registration of an unchanged endpoint legitimately leaves the handle alone).
  • The discovery snapshot publication and the per-URI markReady release run unconditionally. Since nativeCreateOrUpdate, markReady and the default-discovery registration are all idempotent, the unchanged-handle path now claims the identity and restores readiness exactly like the changed-handle path always did.

Regression (testDoRegisterURIResetsReadinessWhenTheHandleIsUnchanged):
A registration subclass whose buildHandle returns null — the exact duplicate-registration shape you probed — must still call markReadyBySelectorIdAndUrl for every URI and publish the DISCOVER_UPSTREAM / UPDATE snapshot. Red on the previous commit (markReady was never invoked), green now.

Verification:

shenyu-admin: 2508 tests green (clean reactor)
checkstyle: clean

No live registry/websocket claim.

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

Reviewed 2a0fc96.

The previously reported same-endpoint REGISTER defect is fixed: doDiscoveryLocal now runs even when buildHandle returns null for an unchanged selector handle, so registration clears persisted readiness/draining markers and publishes the refreshed discovery state. The handler update remains conditional, and existing heartbeat behavior is unchanged. The new regression exercises the shared registration entry point with the unchanged-handle branch and verifies both readiness reset and publication.

The earlier change request is resolved by this revision. Coordinate merging/rebasing with the separate boot-session-fencing PR because both touch registration lifecycle code.

Targeted test report: 160 cases, 0 failures, 0 errors, 0 skipped. Checkstyle, separately executed Apache RAT and git diff --check pass. Validation used ./mvnw and Corretto 21/project target 17; no full-repository/Docker E2E pass is claimed. Required CI must pass before merge.

@Aias00
Aias00 merged commit 755055e into apache:master Oct 30, 2026
56 checks passed
@Aias00 Aias00 added this to the 2.7.2 milestone Nov 13, 2026
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.

2 participants