[
https://issues.apache.org/jira/browse/CAMEL-26816?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18142970#comment-18142970
]
Claus Ibsen commented on CAMEL-26816:
-------------------------------------
Decision on the direction (PR https://github.com/apache/camel/pull/29586):
confirm() marks the key as processed, and this is ON by default. Setting
confirmQuery to null restores the old behaviour, in which confirmed keys stay
locks.
Why:
- Other idempotency stores keep two states: "in progress" is a lease that
expires, so a crashed worker's message is retried; "done" is kept for a
retention period (AWS Lambda Powertools idempotency, NServiceBus outbox,
MassTransit inbox, Stripe idempotency keys). None of them lets a completed key
expire like a lease.
- The current behaviour is inconsistent: a duplicate is rejected while the node
that processed it runs, and accepted once that node stops and lockMaxAgeMillis
passes. The orphan-lock handling is meant for locks of a crashed JVM, not for
completed messages.
- An opt-in would keep a duplicate-processing bug, which only shows on rolling
restarts and scale-in, as the default.
- It matches the parent JdbcMessageIdRepository, and the keep-alive set no
longer grows with every processed key.
The cost: a confirmed key has no age (createdAt is null), so a purge by
createdAt keeps it. The docs should show the purge recipe (an own insertedAt
column, or a confirmQuery that also stores the confirm time). Follow-up for
camel-file: with idempotentEager=false, keys are added but never confirmed.
_Claude Code on behalf of davsclaus_
> camel-sql - JdbcOrphanLockAwareIdempotentRepository: confirm() is a no-op, so
> a processed key is taken over and processed again after its node stops
> ----------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-26816
> URL: https://issues.apache.org/jira/browse/CAMEL-26816
> Project: Camel
> Issue Type: Bug
> Components: camel-sql
> Reporter: shashank
> Assignee: shashank
> Priority: Major
>
> {{JdbcOrphanLockAwareIdempotentRepository}} uses the inherited {{confirm()}},
> which only returns true ({{AbstractJdbcMessageIdRepository.java:207-209}}).
> Its row has no "processed" state (columns {{processorName, messageId,
> createdAt}}, {{JdbcMessageIdRepository.java:41-42}}), so a processed and
> confirmed key is treated like an in-flight lease:
> * While the node runs, the key stays in the keep-alive set
> ({{JdbcOrphanLockAwareIdempotentRepository.java:51}}, added at :123 and :135,
> removed only in {{delete}} at :108). The keep-alive task refreshes its
> {{createdAt}} on every round (:184-198). The set and each keep-alive batch
> grow with every processed message.
> * When the node stops or crashes, the row is no longer refreshed. Once it is
> older than {{lockMaxAgeMillis}}, {{queryForInt}} treats it as absent (:97-98)
> and {{insert}} takes it over (:129-136). {{add()}} returns true for a message
> that was already processed.
> Example, nodes A and B share {{CAMEL_MESSAGEPROCESSED}} with the same
> {{processorName}}:
> # A processes id {{K}}. {{add}} returns true, {{confirm}} does nothing, and A
> keeps refreshing the row.
> # While A runs, a duplicate of {{K}} sent to B is rejected, as expected.
> # A stops (rolling restart, scale-in, redeploy) or crashes.
> # After {{lockMaxAgeMillis}} a duplicate of {{K}} reaches B. With
> {{noop=true}} this is the same file, which is still in the directory. The
> takeover UPDATE matches, {{add}} returns true, and {{K}} is processed a
> second time.
> Found with Specula and confirmed with a test on the real classes, on H2 with
> two repository instances.
> Why it matters: callers that confirm on success hit it with their defaults:
> the Idempotent Consumer EIP (eager), the file consumer with {{noop=true}} or
> {{idempotent=true}}, and {{readLock=idempotent}} with the default
> {{readLockRemoveOnCommit=false}}. The class javadoc and the Orphan Lock
> section of {{sql-component.adoc}} describe the takeover as freeing the lock
> of an instance that crashed. They do not say that processed keys are
> forgotten when their node stops. The parent {{JdbcMessageIdRepository}} keeps
> confirmed keys for good.
> Expected: a confirmed key stays a duplicate after its node stops, and it is
> no longer refreshed by the keep-alive.
> Fix direction, for the maintainers to choose:
> * Should {{confirm()}} remove the key from the keep-alive set and mark the
> row as processed? Without a schema change this could be a {{createdAt}} value
> that the orphan check never treats as old; with one, a state column.
> * Or is the repository meant only for in-flight locks, in which case the docs
> should say so and recommend {{readLockRemoveOnCommit=true}} or the plain
> repository for keys that must stay?
> _Filed with Claude Code on behalf of allthingssecurity._
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
