CAMEL-24242: camel-aws2-athena - stop ignoring thread interruption while polling a query

[oscerd]

Problem

Athena2QueryHelper.interrupted gates both loops that drive startQueryExecution:

if (this.interrupted) {
    LOG.trace("AWS Athena start query execution thread was interrupted, will try no more");
    return false;
}

It is read at Athena2QueryHelper.java:113 (shouldAttempt) and :146
(shouldWait) — and never assigned anywhere in the class. Both guards are
dead code.

How it regressed

This is a silent revert of a deliberate fix, so I want to be explicit about the
history:

• CAMEL-20297 (9e17300779, Otavio Piske, Jan 2024) — "camel-aws2-athena:
  do not swallow interrupted exceptions" — added the interrupt handling:

  try {
      Thread.sleep(this.currentDelay);
  } catch (InterruptedException e) {
      this.interrupted = Thread.interrupted(); // store, then clear, interrupt status
      LOG.trace("...wait thread was interrupted; will return at earliest opportunity");
      Thread.currentThread().interrupt();
  }

• CAMEL-22949 (1b0fca17c0, PR CAMEL-22949: Migrate components from Thread.sleep() to Camel's Task API #21215, Feb 2026) — "Migrate components from
  Thread.sleep() to Camel's Task API" — replaced that block with
  Tasks.foregroundTask()...run(...) and dropped the catch clause, which was
  the only writer of interrupted.

Impact

ForegroundTask.run() does restore the interrupt status
(ForegroundTask.java:124) and return false — but doWait() discarded the
return value. The result is worse than a missed shutdown signal. After the first
interruption:

1. the thread's interrupt status stays set;
2. shouldWait() still returns true, because interrupted is false;
3. the next doWait() reaches Thread.sleep(initialDelay) inside
   ForegroundTask.run(), which throws InterruptedException immediately
   because the status is set, and returns at once;
4. the polling loop therefore spins with no delay at all, issuing
   GetQueryExecution calls as fast as the API allows until waitTimeout
   elapses — and up to maxAttempts × waitTimeout when
   resetWaitTimeoutOnRetry=true.

So interrupting an in-flight query (graceful shutdown, route stop) turns a
2-second-interval poll into a tight busy loop against the Athena API.

Fix

Capture the run() return value and record the interruption:

if (!completed && Thread.currentThread().isInterrupted()) {
    // the task API already restored the interrupt status, so only record it here to let the
    // attempt and wait loops bail out at the earliest opportunity
    this.interrupted = true;
    LOG.trace("AWS Athena start query execution wait thread was interrupted; will return at earliest opportunity");
}

The interrupt status itself is left in place — ForegroundTask already restored
it, which is what CAMEL-20297 wanted the caller to observe.

Test

Athena2QueryHelperTest.doWaitRecordsThreadInterruptionSoTheLoopsStop() interrupts
the calling thread, calls doWait(), and asserts that isInterrupted(),
shouldWait() and shouldAttempt() all report the query loop should stop. It
clears the interrupt status in a finally block so it cannot leak into
neighbouring tests.

Verified it fails on main (Expecting value to be true but was false) and
passes with the fix. Full class: 23/23 green.

assertj-core is added test-scoped for the new assertions; the pre-existing
JUnit assertions in the file are left untouched.

Backport

• camel-4.18.x — affected (carries the CAMEL-22949 migration) → backport opened.
• camel-4.14.x — not affected; it still has the original Thread.sleep()
  with the catch (InterruptedException) block, so there is nothing to fix there.

---

Claude Code on behalf of oscerd

[gnodet]

Clean, well-documented fix for the interrupt-handling regression introduced by the Task API migration (CAMEL-22949). The analysis tracing the issue through CAMEL-20297 → CAMEL-22949 is excellent, and the test correctly validates all three observable effects.

One minor note: adding assertj-core as a new test dependency for 4 boolean assertions could be replaced by JUnit's built-in assertTrue/assertFalse (the rest of the test file already uses them). Not blocking since assertj is managed in the parent POM and widely used elsewhere.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of @gnodet

[github-actions]

🌟 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]

🧪 CI tested the following changed modules:

• components/camel-aws/camel-aws2-athena

---

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

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

⚠️ Modules only in Scalpel (29)

• apache-camel
• camel-allcomponents
• camel-catalog
• camel-catalog-console
• camel-catalog-lucene
• camel-catalog-maven
• camel-catalog-suggest
• camel-componentdsl
• camel-csimple-maven-plugin
• camel-endpointdsl
• camel-endpointdsl-support
• camel-itest
• camel-jbang-core
• camel-jbang-it
• camel-jbang-main
• camel-jbang-plugin-edit
• camel-jbang-plugin-generate
• camel-jbang-plugin-kubernetes
• camel-jbang-plugin-test
• camel-kamelet-main
• camel-launcher
• camel-report-maven-plugin
• camel-route-parser
• camel-yaml-dsl
• camel-yaml-dsl-deserializers
• camel-yaml-dsl-maven-plugin
• coverage
• docs
• dummy-component

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

Modules Scalpel would test (9)

• camel-aws2-athena
• 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

Modules with tests skipped (29)

• apache-camel
• camel-allcomponents
• camel-catalog
• camel-catalog-console
• camel-catalog-lucene
• camel-catalog-maven
• camel-catalog-suggest
• camel-componentdsl
• camel-csimple-maven-plugin
• camel-endpointdsl
• camel-endpointdsl-support
• camel-itest
• camel-jbang-core
• camel-jbang-it
• camel-jbang-main
• camel-jbang-plugin-edit
• camel-jbang-plugin-generate
• camel-jbang-plugin-kubernetes
• camel-jbang-plugin-test
• camel-kamelet-main
• camel-launcher
• camel-report-maven-plugin
• camel-route-parser
• camel-yaml-dsl
• camel-yaml-dsl-deserializers
• camel-yaml-dsl-maven-plugin
• coverage
• docs
• dummy-component

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

All tested modules (38 modules)

• Camel :: AWS2 Athena
• Camel :: All Components Sync point
• Camel :: Assembly
• Camel :: Catalog :: CSimple Maven Plugin (deprecated)
• Camel :: Catalog :: Camel Catalog
• Camel :: Catalog :: Camel Report Maven Plugin
• Camel :: Catalog :: Camel Route Parser
• Camel :: Catalog :: Console
• Camel :: Catalog :: Dummy Component
• Camel :: Catalog :: Lucene (deprecated)
• Camel :: Catalog :: Maven
• Camel :: Catalog :: Suggest
• Camel :: Component DSL
• Camel :: Coverage
• Camel :: Docs
• Camel :: Endpoint DSL
• Camel :: Endpoint DSL :: Support
• Camel :: Integration Tests
• Camel :: JBang :: Core
• Camel :: JBang :: Integration tests
• Camel :: JBang :: MCP
• Camel :: JBang :: Main
• Camel :: JBang :: Plugin :: Edit
• Camel :: JBang :: Plugin :: Generate
• Camel :: JBang :: Plugin :: Kubernetes
• Camel :: JBang :: Plugin :: MCP
• Camel :: JBang :: Plugin :: Route Parser
• Camel :: JBang :: Plugin :: TUI
• Camel :: JBang :: Plugin :: Testing
• Camel :: JBang :: Plugin :: Validate
• Camel :: Kamelet Main
• Camel :: Launcher
• Camel :: Launcher :: Container
• Camel :: YAML DSL
• Camel :: YAML DSL :: Deserializers
• Camel :: YAML DSL :: Maven Plugins
• Camel :: YAML DSL :: Validator
• Camel :: YAML DSL :: Validator Maven Plugin

---

⚙️ View full build and test results

[davsclaus]

Clean bug fix — the regression analysis is thorough and the fix is correctly targeted.

Verified that ForegroundTask.run() catches InterruptedException, restores the interrupt status, and returns false — so the new !completed && Thread.currentThread().isInterrupted() guard correctly bridges the Task API's interrupt signaling back to the Athena2QueryHelper.interrupted field that gates both polling loops.

The test properly sets the interrupt flag before calling doWait(), asserts all three loop-exit conditions (isInterrupted(), shouldWait(), shouldAttempt()), and clears the flag in a finally block to prevent leakage.

This review was generated by an AI agent (Claude Code on behalf of davsclaus) and may contain inaccuracies. Please verify all suggestions before applying.