Ignite cluster observer - #9
Open
NickPadilla wants to merge 7 commits into
Open
Conversation
Diagnose and optionally act on the orphaned-node / split-brain failure modes while structures is on continuum 2.6.x (vertx 4): - VertxClusterCacheConfiguration: REPLICATED FULL_SYNC template for the __vertx.* caches (continuum collects CacheConfiguration beans). With the default PARTITIONED 0-backup caches, a failed node's entries are destroyed with the topology change before vertx-ignite's cleanup listener runs, which breaks the cleanup election and leaves stale subscriptions that fail with 'Not a member of the cluster' - IgniteClusterObserver: logs membership changes with topology context, segmentation events, and stale routing state after node departures (nodeInfo/subs inspection via binary reads, no vertx-ignite compile dependency); watches the server topology against structures.cluster.observer.minimumClusterSize with arm-after-join, grace period, and startup quorum timeout - observe-only by default: shutdown decisions are logged with an [observe-only] prefix; set structures.cluster.observer.shutdownEnabled =true to actually exit for orchestrator replacement Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review of the initial commit showed the cache-config change was based on a wrong premise and would have hurt: - drop VertxClusterCacheConfiguration entirely. Continuum 2.6.0 already contributes a "*" template (PARTITIONED, backups=1, PRIMARY_SYNC) covering the __vertx.* caches, so they were never the 0-backup caches the fix assumed. Ignite matches the longest wildcard, so the narrower __vertx.* template would have silently replaced PRIMARY_SYNC with FULL_SYNC, making every event bus handler registration block on acks from every node - and templates only apply at cache creation, so it would have been inert on rolling deploys regardless. - state the stale-routing mechanism as UNCONFIRMED in the javadoc; the observer exists to capture evidence, not to assume a cause - accept both camelCase and kebab-case property spellings: @value has no relaxed binding, so idiomatic Boot config would have silently left the observer disabled - always log segmentation, never suppressed by the below-minimum report flag (the one-shot listener unsubscribes, so a swallowed log was lost forever), and skip shutdown under the development profile to match continuum's NoOpFailureHandler - remove dead lastObservedServerNodes state (its accessors were not ported) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove every action path so this can be deployed anywhere clustering is on with no behavioral risk: no shutdown, no exit codes, no readiness gating. The only configuration left is structures.cluster.observer.minimumClusterSize (default 1, watchdog off), which controls logging alone. Review fixes carried in: - the stale-route inspection now runs a node-LOCAL bounded ScanQuery on its own thread instead of a cluster-wide iteration on the shared scheduler thread, so it adds no distributed query load during a failure and can never stall topology polling - it is sampled at 5s/20s/60s after a departure, so an in-progress cleanup is distinguishable from a real leak; only the final sample warns - a failed inspection logs at WARN, and a clean result at INFO, so absence of evidence can never be read as evidence of absence - listeners are deregistered before the executors stop, so a departure during shutdown no longer throws RejectedExecutionException into Ignite's discovery thread - segmentation is logged unconditionally and never acted on; continuum's FailureHandler owns what happens to the process Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review found the diagnostics could emit false all-clears, which is worse than no diagnostic. Every finding addressed: - never log an unqualified "clean" result. Missing vertx caches, truncated scans and unreadable keys now produce an explicit "Inconclusive routing inspection ... NOT an all-clear" warning, and every line states the coverage it achieved - read primary AND backup partitions via localEntries instead of a primary-only local ScanQuery: a departure triggers rebalancing, and a backup-held entry is exactly what a primary-only scan would miss - log measured elapsed time per sample instead of the nominal schedule, so a sample that runs late while the cluster is unhealthy does not report a timestamp it did not observe - restore the never-reached-minimum diagnostic (log only): a node that restarts into an ongoing partition forms its own singleton topology that Ignite never merges, and previously produced no output at all - hand membership logging off to the observer thread; Ignite notifies listeners inline on the monitored discovery worker, where a blocked log write could trip SYSTEM_WORKER_BLOCKED and the failure handler - throttle the "topology is not queryable" warning (it could repeat every 10s forever), report the underlying exception instead of a bare serverNodes=-1 sentinel, fix the scan-cap off-by-one, and log shutdown-interrupted inspections at DEBUG so ordinary deploys stay quiet - plumb structures.cluster.observer.minimumClusterSize through the Helm configmap and values so the topology reporting is reachable in k8s Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Final pre-release pass. The observer must be able to run for days and still be believed, so every reported number is now defensible and every Ignite interaction is bounded. Counting and ownership: - count an entry as stale only on the node whose affinity function says it currently owns that partition as PRIMARY. That restores the once-per-entry invariant (localEntries(PRIMARY,BACKUP) had started double counting across nodes) and excludes partitions being rebalanced away, which still hold their old contents and produced phantom findings during rolling restarts. Backup-held copies are counted and reported separately, never mixed into the summable total. Bounded and leak-free: - every Ignite read is time-bounded (async containsKey with timeout, a deadline inside the scan), so the partition-map-exchange stall this class exists to document can no longer silence it - resolve cache handles via cacheNames() so an unknown name cannot trigger a blocking cluster-wide dynamic cache start - close the scan iterator on every path, including the truncation break - shutdown() instead of shutdownNow(): interrupting a thread inside an Ignite operation risked this node leaving as FAILED rather than LEFT Reporting fidelity: - never-reached-minimum now warns first and only escalates to ERROR once slow cluster formation is implausible, repeats at a low rate so a days-old one-shot line is not the only record, and is explicitly retracted when the topology later reaches the minimum - membership and segmentation are logged inline again, from data carried on the event itself (no cluster calls on the discovery thread), so the lines survive a JVM halt moments later - the nodeInfo and subs halves are independent again: a missing subs cache no longer costs the nodeInfo verdict, the direct evidence of a failed cleanup election - throttle the catch-all error path Configuration moved into StructuresProperties as cluster-observer.*, so it ships with configuration metadata and validation like every other structures setting, and the Helm value is a top-level clusterObserver block (the env var binding was verified against Spring's relaxed binder). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Chasing an exact, cluster-summable stale-entry count kept trading one defect for another, and precision was never what this needs: a transient failure just means the next sample retries, while a real problem shows up as the same complaint repeating. So the inspection is now defensive by construction and self-contained - it cannot affect clustering. - inspections run on a small bounded pool instead of a single thread, so a read wedged inside Ignite can no longer silence every later inspection for the life of the pod; when all threads are busy the skipped inspection is logged, which is itself evidence of a hang - drop the per-entry affinity ownership check. It could silently discard entries when the topology moved mid-scan (producing exactly the false all-clear the class forbids) and left backup-only findings with an empty address list. Counts are now plainly this node's local view and documented as overlapping across pods - catch Throwable, not Exception, around inspections: an Error from Ignite internals iterating a partition being evicted would otherwise vanish into the executor as a silence that reads like a clean result - always report why an inspection was incomplete, at INFO for early samples (expected during a rebalance) and WARN on the final one, so a failure is never invisible at production log levels nor alarming when it is routine - report the below-minimum condition hourly rather than once per process, so a node orphaned days ago is still visible in a recent log window - stop resetting the unqueryable-topology throttle on success, which made it useless against an alternating failure pattern - move the Helm value under properties.structures.clusterObserver, where every other STRUCTURES_* setting lives, so an override placed there is not silently ignored Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The config surface was arbitrary: when a condition starts being reported was a property, but how often it repeats - and the routing sample schedule - were hardcoded. Since this ships in an immutable release and may run for days, promote the three tunables most likely to need adjusting in a specific environment: - repeatBelowMinimumEveryMs and repeatNeverReachedEveryMs (both 1h), pairing with the existing report-after settings - staleRouteSampleDelaysMs (5s/20s/60s): if cleanup legitimately takes longer than the window in a given environment, every final sample would warn, and that can now be widened without a release. Falls back to the defaults if configured empty, so the inspection can never be silently disabled Defaults are unchanged, so behavior is identical unless set. The remaining constants (poll interval, throttles, pool sizing, address sample limit) stay internal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.