Hazelcast impl - #1011
Merged
rsandell merged 94 commits intoJul 27, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a Hazelcast-backed coordination mode to allow the Gerrit Trigger Plugin’s BuildMemory state (and related coordination concerns) to work in distributed Jenkins/CloudBees HA/HS deployments.
Changes:
- Added a Hazelcast-based
BuildMemoryStorageimplementation plus supporting Hazelcast serialization/config/entry-processors. - Extended the coordination SPI/factory to support additional strategies (event + notification claiming) and provider lifecycle (initialize/shutdown).
- Added/updated test infrastructure and integration tests, plus a Maven profile to run the test suite with Hazelcast enabled.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/BuildCancellationHazelcastIntegrationTest/common/gerrit-trigger.xml | Adds LocalData config fixture for Hazelcast cancellation integration tests. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/spec/VoteSameTopicTest.java | Clears Hazelcast maps in teardown to avoid cross-test pollution. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/spec/SpecGerritTriggerHudsonTest.java | Clears Hazelcast maps in teardown to avoid cross-test pollution. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/spec/ParameterModeJenkinsTest.java | Adds an @After hook to clear Hazelcast state between tests. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/replication/ReplicationQueueTaskDispatcherTest.java | Stabilizes time-based assertions and clears Hazelcast state before/after the class. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/BuildCancellationIntegrationTest.java | Clears Hazelcast state in teardown to prevent test pollution when Hazelcast is active. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/hudsontrigger/BuildCancellationHazelcastIntegrationTest.java | Adds Hazelcast-mode-specific build cancellation integration tests and Hazelcast test rule usage. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastTestRule.java | Adds a JUnit rule to initialize/shutdown Hazelcast and set coordination-mode system property. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastTestListener.java | Adds a surefire listener to initialize Hazelcast once for the entire Hazelcast test profile run. |
| src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastTestHelper.java | Adds helper methods to clear Hazelcast maps and inspect BuildMemory map size for tests. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/storage/LocalBuildMemoryStorage.java | Adds setCancelling(...) and eventsMatch(...) for local storage semantics. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/spi/NotificationClaimStrategy.java | Refactors notification claiming to a fluent withClaim(...) API with chainable handlers. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/spi/EventClaimStrategy.java | Introduces event-claiming SPI for distributed duplicate-prevention. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/spi/CoordinationModeProvider.java | Extends SPI to support configured-mode lookup plus provider lifecycle and event claim strategy creation. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/spi/BuildMemoryStorage.java | Extends storage SPI with setCancelling(...) and storage-defined event equivalence (eventsMatch). |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/PluginImpl.java | Initializes/shuts down coordination providers during plugin start/stop. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/model/BuildMemory.java | Delegates cancelling flag persistence to storage; delegates event comparison to storage via eventsMatch. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/LocalNotificationClaimStrategy.java | Implements the new fluent notification claim API for local mode. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/LocalEventClaimStrategy.java | Implements event claiming for local mode using the new fluent API. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/LocalCoordinationProvider.java | Adds event claim strategy and provider lifecycle methods to local provider. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/TriggeredProcessor.java | Adds Hazelcast EntryProcessor for atomic “triggered” updates. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/SetUnsuccessfulMessageProcessor.java | Adds Hazelcast EntryProcessor for atomic unsuccessful-message updates. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/SetCustomUrlProcessor.java | Adds Hazelcast EntryProcessor for atomic custom-URL updates. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/SetCancellingProcessor.java | Adds Hazelcast EntryProcessor for atomic “cancelling” flag updates. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/PolymorphicEventTypeAdapter.java | Adds Gson adapter to serialize/deserialize polymorphic GerritTriggeredEvent payloads. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/MemoryImprintDataSerializer.java | Adds Hazelcast Compact serializer for MemoryImprintData. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/MemoryImprintData.java | Adds DTO for storing memory imprints in Hazelcast via Compact serialization. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastNotificationClaimStrategy.java | Adds Hazelcast-backed distributed notification claim strategy with TTL. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastManager.java | Adds lifecycle management for embedded Hazelcast member initialization/shutdown. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastInstanceProvider.java | Adds a singleton provider for the Hazelcast instance. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastEventClaimStrategy.java | Adds Hazelcast-backed event claiming to avoid duplicate event processing across replicas. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastCoordinationProvider.java | Adds Hazelcast coordination provider implementation (storage + claim strategies + lifecycle). |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastConfig.java | Adds Hazelcast network/discovery/serialization configuration builder. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/HazelcastBuildMemoryStorage.java | Adds Hazelcast-backed BuildMemoryStorage implementation using IMap + EntryProcessors. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/EventIdentifier.java | Adds deterministic event ID generation for distributed keys/claims (with fallback). |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/EventClaimSerializer.java | Adds Hazelcast Compact serializer for EventClaim. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/EventClaim.java | Adds DTO representing an event claim stored in Hazelcast. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/EntryDataSerializer.java | Adds Hazelcast Compact serializer for EntryData. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/EntryData.java | Adds DTO representing per-job build entry data for Hazelcast storage. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/BuildStartedProcessor.java | Adds Hazelcast EntryProcessor for atomic “build started” updates. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/BuildMemoryKey.java | Adds Hazelcast map key based on deterministic event IDs. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/BuildCompletedProcessor.java | Adds Hazelcast EntryProcessor for atomic “build completed” updates. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/hazelcast/BuildCancelledProcessor.java | Adds Hazelcast EntryProcessor for atomic “build cancelled” updates. |
| src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/coordination/CoordinationModeFactory.java | Extends factory to create event claim strategy in addition to storage + notification claim strategy. |
| pom.xml | Adds Hazelcast dependency and a Maven profile to run tests with Hazelcast coordination enabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 22, 2026
rsandell
reviewed
May 24, 2026
rsandell
reviewed
May 24, 2026
rsandell
reviewed
May 24, 2026
rsandell
reviewed
May 24, 2026
rsandell
reviewed
May 24, 2026
Update README files
EventIdGenerator is a shared utility (used by both claim strategies, not just BuildMemoryKey), so the rename clarifies its role. BuildMemoryKey had no real callers — HazelcastBuildMemoryStorage deliberately uses raw String keys instead to avoid Hazelcast classloader issues.
Relocate PipelineAbortHelper (and its test) out of coordination.hazelcast into hudsontrigger, widening it from package-private to public so HazelcastBuildMemoryStorage can use it across the package boundary.
This was referenced Jul 24, 2026
rsandell
reviewed
Jul 26, 2026
| // causing some project entries to be lost from BuildMemory (which breaks cancellation logic). | ||
| // Note: EntryProcessor is NOT used here because in client mode the processor class would need | ||
| // to exist on the Hazelcast sidecar member's classpath, causing ClassNotFoundException. | ||
| if (!tryLockWithTimeout(map, key)) { |
Member
There was a problem hiding this comment.
When did this change?
I thought we had agreed on do it in the fluent style? Like
withLock(() -> {
//do it
}).onFailure(() -> logger.error("Could not acquire distributed lock for key {} within {}s - skipping triggered()", key, LOCK_ACQUIRE_TIMEOUT_SECONDS);)
Member
There was a problem hiding this comment.
I can't find if we've had any discussion around changing that for the internals here? Maybe we have and my memory is betraying me?
This isn't a showstopper now, but it had beeen nice if we had kept the same locking style throughout.
rsandell
reviewed
Jul 26, 2026
| * | ||
| * @param hz the Hazelcast instance whose cluster membership should be observed | ||
| */ | ||
| private void waitForClusterFormation(HazelcastInstance hz) { |
Member
There was a problem hiding this comment.
question, wouldn't this break or at least interfere with the sollution we discussed around event replay during statup?
There was a problem hiding this comment.
Not really, although it is worth an explanation.
waitForClusterFormation() runs inside initializeCoordinationProviders(), which completes before
GerritServer.start()/startConnection() in PluginImpl.start(). So the Gerrit SSH connection, and with it
connectionEstablished() (which triggers missed-events playback), waits on this.
Given the sidecar-per-replica topology this class's javadoc already documents (sidecar discovery is single-digit
seconds, Jenkins' own boot is tens of seconds), this shouldn't cause a practical problem for that topology at cold start. Worth documenting as a known, small risk rather than blocking on it. The guarantee is scoped to startup timing, not every later reconnect, but that's already true of this design generally, not something new here.
On scope: this PR doesn't add cross-replica coordination for missed-events catch-up. Each replica
still runs its own local, uncoordinated playback on reconnect, same as today. Proper coordination (per-instance
timestamp files, a shared watermark, mutual exclusion via a distributed lock) is being developed on
distriubuted-missed-events and will follow in a later PR once rebased.
One item from that branch worth a second opinion: its coordination lock uses a 300s lease documented as a crash-only safety net, assumed not to fire in the normal path. The scenario it's built for (every replica
simultaneously unable to process events during a full outage) is also the case most likely to produce a catch-up
backlog that takes longer than 5 minutes to fetch and trigger.
@rsandell, is 300s realistic, or should we make it configurable?
rsandell
reviewed
Jul 26, 2026
| // event?" - it never asks the reverse. Left unguarded, a late-arriving | ||
| // older patchset's own build runs to completion fully unsuppressed | ||
| // whenever cross-replica event delivery reorders patchset arrival | ||
| // (confirmed to happen on mc3 - see the HZ-104 cross-node cancellation |
Member
There was a problem hiding this comment.
Reference to HZ-104 which I gueess is outsside the repo?
rsandell
reviewed
Jul 26, 2026
| // cancel" - it is not itself proof the entry is done. Excluding it here | ||
| // let relocated-but-not-yet-restarted entries dodge cancellation entirely | ||
| // whenever a newer patchset arrived during the relocation window (the | ||
| // HZ-006/HZ-104 mc3 race). |
rsandell
reviewed
Jul 26, 2026
| * {@code newEvent} - it correctly refuses to do so when {@code runningEvent} is actually | ||
| * newer, but nothing then cancels {@code newEvent} itself in that case. Cross-replica event | ||
| * delivery can deliver a newer patchset's event to some replica before an older one reaches | ||
| * any replica at all (confirmed on {@code mc3} - see the HZ-104 cross-node cancellation race |
Member
There was a problem hiding this comment.
References to mc3 and HZ-104 . Needs to be "locateable" or perhaps if it can be rewritten without them?.
rsandell
reviewed
Jul 26, 2026
| * via {@link BuildMemoryStorage#eventsMatch(GerritTriggeredEvent, GerritTriggeredEvent)}. | ||
| * This respects the abstraction boundary: | ||
| * <ul> | ||
| * <li><strong>Local mode:</strong> Uses identity comparison (==)</li> |
Member
There was a problem hiding this comment.
I've mentioned this before that == is nott a good thing to do since eveents can be deserialized from disk ettc. and won't be the same instance even though tthey are tthe same event.
There was a problem hiding this comment.
Stale javadoc comment only. Fixed with ffec861
rsandell
reviewed
Jul 26, 2026
| * </ul> | ||
| * Unlike {@link #isCancelled()}, setting this flag does NOT also set | ||
| * {@link #setBuildCompleted(boolean)}, preserving the IMap entry for cross-instance | ||
| * new-patchset abort scenarios (HZ-004). |
rsandell
reviewed
Jul 26, 2026
Comment on lines
+64
to
+103
| public void testPipelineBlockedAtSemaphoreReturnsFalse() throws Exception { | ||
| WorkflowJob job = jenkins.createProject(WorkflowJob.class, "pipeline-semaphore"); | ||
| job.setDefinition(new CpsFlowDefinition( | ||
| "semaphore 'wait'\n" | ||
| + "echo 'done'", true)); | ||
|
|
||
| WorkflowRun run = job.scheduleBuild2(0).waitForStart(); | ||
|
|
||
| // Wait until the semaphore step is reached — at this point CPS is fully started | ||
| SemaphoreStep.waitForStart("wait/1", run); | ||
|
|
||
| assertFalse("Pipeline blocked at semaphore should report CPS started", | ||
| PipelineAbortHelper.isPipelineNotYetStarted(run)); | ||
|
|
||
| // Unblock and let the build finish cleanly | ||
| SemaphoreStep.success("wait/1", null); | ||
| jenkins.waitForCompletion(run); | ||
| jenkins.assertBuildStatusSuccess(run); | ||
| } | ||
|
|
||
| /** | ||
| * A Pipeline build that has been interrupted after it started should still | ||
| * report false — it is past initialisation, so delivery was correct. | ||
| */ | ||
| @Test | ||
| public void testAbortedPipelineReturnsFalse() throws Exception { | ||
| WorkflowJob job = jenkins.createProject(WorkflowJob.class, "pipeline-aborted"); | ||
| job.setDefinition(new CpsFlowDefinition( | ||
| "semaphore 'wait-abort'\n" | ||
| + "echo 'done'", true)); | ||
|
|
||
| WorkflowRun run = job.scheduleBuild2(0).waitForStart(); | ||
| SemaphoreStep.waitForStart("wait-abort/1", run); | ||
|
|
||
| // Abort while it's at the semaphore (CPS has started) | ||
| assertFalse(PipelineAbortHelper.isPipelineNotYetStarted(run)); | ||
|
|
||
| run.getExecutor().interrupt(Result.ABORTED); | ||
| jenkins.waitForCompletion(run); | ||
| } |
Member
There was a problem hiding this comment.
Itt looks to me like these two are testing the same thing?
There was a problem hiding this comment.
Good catch. Changed test to test condition after the abort phase with 892f175
rsandell
reviewed
Jul 26, 2026
| META-INF/services/org.junit.platform.launcher.TestExecutionListener). This provides | ||
| the localhost:5702 endpoint that the plugin client connects to, with no external | ||
| Kubernetes or TCP infrastructure required. | ||
| --> |
Member
There was a problem hiding this comment.
So thiss means that there will be no hazelcastt specific tests at all during norrmal CI builds?
I remember we discussed that we'll need at least one smoke test durng the normal run to have some indication that future work doesn't break the hazelcast coordination mode.
Member
|
just some smaller nits left, I'll bee available tomorrow as well. |
Replaces the repeated tryLockWithTimeout/try-finally pattern with a withLock(action).onFailure(...) helper across HazelcastBuildMemoryStorage, to keep the locking style consistent.
The assertion ran before interrupt()/waitForCompletion(), so it only duplicated testPipelineBlockedAtSemaphoreReturnsFalse's pre-abort check and never verified the aborted/completed state its name and javadoc claimed to cover.
The test-hazelcast Maven profile is opt-in only, and HazelcastTestRule uses Assume.assumeTrue to skip BuildCancellationHazelcastIntegrationTest whenever it isn't active. As a result, a normal `mvn test` run never exercises Hazelcast coordination mode - the test class is picked up by surefire but every test in it is silently skipped. HazelcastCoordinationSmokeTest closes that gap without requiring the profile. It starts its own embedded Hazelcast server and sets the coordination-mode/client-address system properties from a @ClassRule, which JUnit4 guarantees runs before JenkinsRule regardless of field order (the same ordering problem HazelcastTestRule sidesteps by requiring the properties to be set before the JVM starts via the profile). It then asserts CoordinationModeFactory actually selected HazelcastBuildMemoryStorage - not a silent fallback to local mode - and triggers a real Gerrit patchset event end-to-end, verifying the build completes successfully over the live Hazelcast client/server connection. This exercises build-memory storage, event claiming, and notification claiming, giving CI a fast signal that future changes haven't broken Hazelcast coordination, without the cost/flakiness surface of the full cancellation-race suite, which remains opt-in behind -Ptest-hazelcast.
sboardwell
force-pushed
the
hazelcast-impl
branch
from
July 27, 2026 08:08
903f586 to
5887ec3
Compare
rsandell
approved these changes
Jul 27, 2026
rsandell
left a comment
rsandell
left a comment
Member
There was a problem hiding this comment.
I am happpy!
But tests are timing out on CI?
Member
|
So the smoke test is failing somehow on the java 21 branch, and that causes the 17 branch to be killed and causes all the other timeeoutts is my current guess. |
On it |
HazelcastCoordinationSmokeTest copied SERVER_WAIT=2000 from the BuildCancellation tests, but unlike those it runs a live Hazelcast client in the same JVM. On a resource-constrained agent that extra CPU contention slows the Gerrit SSH handshake so 'gerrit stream-events' arrives after the 2s window - the connection is healthy and the command does fire, just late (it missed by ~50ms on CI). Reproduced locally under single-core contention: fails at 2s, passes at 20s. 20s matches GerritServerSshServerTest and GerritTriggerApiTest; waitForCommand returns as soon as the command appears, so the larger ceiling costs nothing on a fast machine.
Member
|
@sboardwell maybe I beat yoou to it :) |
Nice 🙂 |
rsandell
merged commit Jul 27, 2026
365d808
into
jenkinsci:distributed-storage
15 of 16 checks passed
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
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.
Add Hazelcast distributed storage implementation
This PR implements distributed BuildMemory storage using Hazelcast, enabling the Gerrit Trigger Plugin for distributed build memory.
Changes
Core Implementation
BuildMemoryStorageabstract class with two implementations:LocalBuildMemoryStorage: TreeMap-based storage for standalone Jenkins (existing behavior)HazelcastBuildMemoryStorage: Hazelcast IMap-based storage for distributed JenkinsPoints
BuildMemoryStorageinterface, remaininginfrastructure-agnostic
Testing done
CoordinationModeProvider extension is present
Submitter checklist