gnodet-bot commented on code in PR #28206:
URL: https://github.com/apache/camel/pull/28206#discussion_r4336464997
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc:
##########
@@ -11,6 +11,30 @@ Note that manual migration is still required.
See the xref:camel-upgrade-recipes-tool.adoc[documentation] page for details.
====
+== Upgrading from 4.18.5 to 4.18.6
+
+=== camel-azure-eventhubs - per-partition checkpoint counters and timer;
checkpoint window boundary fix
+
+Several fixes from 4.22.x have been brought forward to 4.18.6 to give this
change a sound base.
+
+`checkpointBatchSize` and `checkpointBatchTimeout` now count events per
partition, not across all
+partitions. Each partition has its own `AtomicInteger` counter and its own
`ScheduledFuture` timer.
+A checkpoint on one partition no longer resets the count of another. Before
this change, the shared
+counter mixed events across partitions, so the batch size was reached across
all partitions combined
+and a checkpoint on a busy partition could reset the count for a quiet one.
+
+The `CamelAzureEventHubsCheckpointUpdatedBy` header can now be `timeout`. With
`checkpointBatchTimeout`,
+the consumer now starts a new checkpoint window when the timer of the current
window has run, and no
+longer when the wall clock passes the time the timer was scheduled for.
Before, an event that completed
+in the millisecond in which the timer ran (or while the timer was taking the
count) was counted in the
+window whose count had already been taken: it was checkpointed only with the
next event, and when the
+partition stayed quiet it was delivered again after a restart. Routes that
read this header may now see
+`timeout` more often.
+
+Checkpoint failures from the timer are now logged at `WARN` level with the
full stack trace instead of
+at `DEBUG` level with the message only. Checkpoint failures triggered by batch
size are still logged at
+`WARN`.
Review Comment:
⚠️ **Inaccurate**: In the base branch, batch-size checkpoint failures were
also logged at `DEBUG` (same as the timer path). Both were changed to `WARN` by
this PR — this is not a "still at WARN" case.
```suggestion
Checkpoint failures from the timer and from the batch-size trigger are now
logged at `WARN` level with the full stack trace instead of
at `DEBUG` level with the message only.
```
##########
components/camel-azure/camel-azure-eventhubs/pom.xml:
##########
@@ -72,5 +72,16 @@
<artifactId>commons-lang3</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.assertj</groupId>
+ <artifactId>assertj-core</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.awaitility</groupId>
+ <artifactId>awaitility</artifactId>
+ <version>${awaitility-version}</version>
+ <scope>test</scope>
+ </dependency>
Review Comment:
🔧 **Redundant**: `awaitility` is already a direct `<dependency>` in
`components/pom.xml` (inherited by all component submodules). No other
`camel-azure` submodule that uses awaitility re-declares it. This block can be
removed — `assertj-core` above must stay (it's not in `components/pom.xml`).
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]