Ignite cluster observer - #9
Open
NickPadilla wants to merge 5 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>
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.