Skip to content

CAMEL-24006: Fix flaky JpaPollingConsumerLockEntityTest#24612

Merged
gnodet merged 1 commit into
apache:mainfrom
gnodet:fix/CAMEL-24006-flaky-JpaPollingConsumerLockEntityTest
Jul 12, 2026
Merged

CAMEL-24006: Fix flaky JpaPollingConsumerLockEntityTest#24612
gnodet merged 1 commit into
apache:mainfrom
gnodet:fix/CAMEL-24006-flaky-JpaPollingConsumerLockEntityTest

Conversation

@gnodet

@gnodet gnodet commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Claude Code on behalf of gnodet

Fixes the flaky JpaPollingConsumerLockEntityTest which was failing on CI ~5.9% of the time (testPollingConsumerWithoutLock: 9 failures + 7 flaky out of 270 runs over the last 10 days).

Root cause: The test relied on two concurrent JPA polling consumers naturally racing to update the same versioned entity without pessimistic locking. Under CI load, one request could complete before the other started its read, avoiding the optimistic lock conflict entirely. When no OptimisticLockException occurred, the assertion on mock:error failed.

Fix: Use a CyclicBarrier in the enrichment strategy for the not-locked route. Both threads must complete the entity read (and in-memory mutation) before either can proceed to the JPA write. This guarantees both hold stale version references, making the OptimisticLockException deterministic rather than probabilistic.

  • Added a CyclicBarrier(2) field to the test class
  • Created a separate enrichment strategy for the not-locked route that waits on the barrier after the read
  • Added an explicit 20-second timeout to the not-locked test's MockEndpoint.assertIsSatisfied() call

Test plan

  • mvn test -pl components/camel-jpa -Dtest=JpaPollingConsumerLockEntityTest passes consistently (5/5 runs locally)
  • CI build passes

🤖 Generated with Claude Code

The testPollingConsumerWithoutLock test was flaky because it relied on
two concurrent JPA polling consumers naturally racing to update the
same versioned entity, which is non-deterministic. Under CI load, one
request could complete before the other started its read, avoiding
the optimistic lock conflict entirely.

Fix: use a CyclicBarrier in the enrichment strategy for the not-locked
route so both threads must complete the entity read before either can
proceed to the write. This guarantees both hold stale version references,
making the OptimisticLockException deterministic.

Also added an explicit 20-second timeout to the not-locked test's
assertIsSatisfied call to prevent indefinite hangs under CI contention.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • components/camel-jpa

🔬 Scalpel shadow comparison — Scalpel: 1 tested, 0 compile-only — current: 9 all tested

Maveniverse Scalpel detected 1 affected modules (current approach: 9).

Modules only in current approach (8)
  • camel-jbang-mcp
  • camel-jbang-plugin-mcp
  • camel-jbang-plugin-route-parser
  • camel-jbang-plugin-tui
  • camel-jbang-plugin-validate
  • camel-launcher-container
  • camel-yaml-dsl-validator
  • camel-yaml-dsl-validator-maven-plugin

Skip-tests mode would test 1 modules (1 direct + 0 downstream), skip tests for 0 (generated code, meta-modules)

Modules Scalpel would test (1)
  • camel-jpa

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (9 modules)
  • Camel :: JBang :: MCP
  • Camel :: JBang :: Plugin :: MCP
  • Camel :: JBang :: Plugin :: Route Parser
  • Camel :: JBang :: Plugin :: TUI
  • Camel :: JBang :: Plugin :: Validate
  • Camel :: JPA
  • Camel :: Launcher :: Container
  • Camel :: YAML DSL :: Validator
  • Camel :: YAML DSL :: Validator Maven Plugin

⚙️ View full build and test results

@gnodet gnodet marked this pull request as ready for review July 12, 2026 01:49
@gnodet gnodet requested review from davsclaus and oscerd July 12, 2026 01:49

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review — solid fix that makes a probabilistic race condition deterministic.

Root cause: Correct — without synchronization, one asyncRequestBodyAndHeaders could complete its entire read-mutate-write cycle before the other starts its read, meaning the second thread reads the already-committed version and no OptimisticLockException occurs.

Fix: The CyclicBarrier(2) approach is textbook for forcing concurrent interleaving in tests:

  1. Both threads enter pollEnrich and read the entity (same stale version)
  2. Both threads hit the barrier inside notLockedEnrichStrategy.aggregate() — neither proceeds until both have read
  3. Both proceed to to("jpa://...") — the second commit triggers OptimisticLockException deterministically

Good design decisions:

  • Separate enrichment strategy for the not-locked route rather than modifying the shared one — the locked route doesn't need the barrier since pessimistic locking handles the conflict at the DB level
  • 10s barrier timeout — long enough for CI, prevents test from hanging if one thread fails before reaching the barrier
  • 20s assertIsSatisfied timeout — accommodates slow CI
  • Assertion change to expectedMessageCount(1) + message(0).body().isEqualTo(...) is functionally equivalent and fine

Claude Code self-review on behalf of @gnodet

@gnodet gnodet added the test label Jul 12, 2026
@gnodet gnodet self-assigned this Jul 12, 2026
@gnodet gnodet added this to the 4.22.0 milestone Jul 12, 2026
@gnodet gnodet merged commit 01a855a into apache:main Jul 12, 2026
5 checks passed
@gnodet gnodet deleted the fix/CAMEL-24006-flaky-JpaPollingConsumerLockEntityTest branch July 12, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants