Introduce `ShareAckMode` enum for share consumers by sobychacko · Pull Request #4376 · spring-projects/spring-kafka

sobychacko · 2026-03-29T02:49:39Z

The 4.0 boolean explicitShareAcknowledgment only controlled whether the listener received a non-null acknowledgment object. It had no effect on Kafka's share.acknowledgement.mode, so acknowledge() would throw IllegalStateException when the factory used the Kafka default (implicit mode).

ContainerProperties.ShareAckMode (EXPLICIT, MANUAL, IMPLICIT) maps each use case to a clear name, following the same pattern as regular containers disabling auto.commit. For EXPLICIT and MANUAL the container now enforces share.acknowledgement.mode=explicit via consumer override properties, regardless of factory configuration.

Other related changes:

RecordDeserializationException handler now guards acknowledge() against IMPLICIT mode
createShareConsumer variant added to ShareConsumerFactory for consumer-level overrides
4.0 dead code and naming issues cleaned up

Deprecated methods keep the 4.0 API intact.
Reference docs and What's New include a migration guide. Test changes.

sobychacko · 2026-03-29T02:52:56Z

The 4.0 explicitShareAcknowledgment boolean never touched Kafka's share.acknowledgement.mode. Kafka defaults that to implicit, which means consumer.acknowledge() throws IllegalStateException. The container called it unconditionally, so the feature only worked if the user had manually configured share.acknowledgement.mode=explicit in the factory.

ShareAckMode (EXPLICIT, MANUAL, IMPLICIT) replaces the boolean, following the same pattern as regular containers disabling auto.commit. For EXPLICIT and MANUAL, the container now enforces share.acknowledgement.mode=explicit via a new createShareConsumer override, so the correct Kafka protocol mode is always active regardless of factory config.

share.acknowledgement.mode=implicit in factory config was non-functional in 4.0. It's now detected, warned about, and overridden. Users who want true implicit mode should set ShareAckMode.IMPLICIT on ContainerProperties.

The 4.0 boolean `explicitShareAcknowledgment` only controlled whether the listener received a non-null acknowledgment object. It had no effect on Kafka's `share.acknowledgement.mode`, so `acknowledge()` would throw `IllegalStateException` when the factory used the Kafka default (implicit mode). `ContainerProperties.ShareAckMode` (EXPLICIT, MANUAL, IMPLICIT) makes the modes explicit, with EXPLICIT as the default — the same philosophy as regular containers disabling auto.commit and owning the commit lifecycle themselves. For EXPLICIT and MANUAL the container now enforces `share.acknowledgement.mode=explicit` via consumer override properties, regardless of factory configuration. Other related fixes: - `RecordDeserializationException` handler now guards `acknowledge()` against IMPLICIT mode - `createShareConsumer` variant added to `ShareConsumerFactory` for consumer-level overrides - 4.0 dead code and naming issues cleaned up Deprecated methods keep the 4.0 API intact. Reference docs and What's New include a migration guide. Adding tests to verify the changes. Signed-off-by: Soby Chacko <soby.chacko@broadcom.com>

artembilan

I think that's enough for now.
Let's work out a common ground and come together on the same page first!

Thank you for doing this, Soby!

artembilan · 2026-03-30T16:45:00Z

+[[x41-share-ack-mode]]
+=== Share Consumer Acknowledgment Modes
+
+The boolean `setExplicitShareAcknowledgment(boolean)` property on `ContainerProperties` has been replaced by the `ShareAckMode` enum, which clearly names the three distinct acknowledgment modes:


That is too much info for whats-new.adoc.
Why change pattern and provide all of that here, but not in the target chapter?

Ok, will move to the chapter.

artembilan · 2026-03-30T16:57:12Z

+	 * @since 4.1
+	 * @see #setShareAckMode(ShareAckMode)
+	 */
+	public enum ShareAckMode {


Feels like a decision to reuse ContainerProperties for ShareConsumer was wrong direction.
Now we give end-user confusion with existing AckMode, which apparently is not used ShareKafkaMessageListenerContainer.
And there are probably many other conflicting options not used here or there.
Not saying that it should be revised now, but something to keep in mind if we'd like to still keep our API end-user friendly.

Ya, we have time to refine it before RC1.

The problem is that we need to distinguish two kinds of containers if we introduce a properties class just for share consumers.

Artem - as we discussed yesterday, we will try to tackle this as a follow up item as this involves a lot of moving parts.

artembilan · 2026-03-30T17:30:53Z

+ * <li><strong>EXPLICIT</strong> (default): Container-managed. The container sends ACCEPT after
+ * successful processing and delegates error handling to the {@link ShareConsumerRecordRecoverer}
+ * (default: REJECT). Equivalent to disabling {@code auto.commit} on a regular consumer.</li>
+ * <li><strong>MANUAL</strong>: Listener-managed. The listener must acknowledge each record via


I don't say that there is something wrong with the logic described here, but I wonder if that has crossed your mind that technically there is no difference between EXPLICIT and MANUAL.
Let's imaging the environment if we change the logic only for two EXPLICIT and IMPLICIT!
The EXPLICIT is MANUAL at the same time.
So, we always provide ack callback to end-user listener in EXPLICIT mode.
And if that callback was not called from end-user code, the container does that, respectively, for both success and error cases.

What do I miss, please?
What is the real driver for the MANUAL if it still per record?

Hey Artem, thanks for the thoughtful feedback on this particular issue. I hear your arguments there, but I'd like to make the case for keeping both modes. The distinction is actually the same one we already draw for regular consumers — AckMode.BATCH (container owns everything, listener is unaware) vs AckMode.MANUAL (listener owns it, must call acknowledge()). There's no auto-fallback in regular consumer MANUAL mode either, and I think that strictness is intentional and valuable.

For share consumers:
• EXPLICIT-- listener is completely unaware of acknowledgment. The acknowledgment parameter is null, no API in scope, no decision to make. Container handles success and error outcomes.
• MANUAL-- listener owns every outcome. Non-null acknowledgment, and if you don't call it, the next poll() throws — the Kafka client enforces the contract, not just the docs.

The auto-fallback model silently auto-accepts a record when a listener forgets to call acknowledge(). In the strict MANUAL model that's a fast-fail -- the blocked poll makes the bug immediately obvious. Collapsing the two for share consumers while keeping them separate for regular consumers would be an inconsistency in the framework, no?

I think keeping both gives users a clear, unambiguous contract that aligns with how Spring Kafka already thinks about acknowledgment.

artembilan · 2026-03-30T17:36:25Z

+								+ "ShareAckMode.IMPLICIT on the container properties instead.");
+					}
+				}
+				catch (UnsupportedOperationException ex) {


Why do we need to catch it here?
If that is not implemented in the provided ShareConsumerFactory, then better to fail for that reason.
This adds extra unnecessary noise into our code and even make it worse: this catch-n-log hides the problem with invoked method.

good idea. will fix.

- Simplify whats-new.adoc and move detailed migration guide to ref docs - Remove try-catch around `getConfigurationProperties()` - let it fail if not implemented Signed-off-by: Soby Chacko <soby.chacko@broadcom.com>

artembilan

Looks cool, but hard to digest.

Let's hope my review is not very destructive!

Thank you for doing this!

artembilan · 2026-03-31T18:31:57Z

+			assertThat(redeliveryLatch.await(5, TimeUnit.SECONDS))
+					.as("Record should be archived (REJECT) and not redelivered")
+					.isFalse();
+			assertThat(deliveryCount.get()).isEqualTo(1);


The last two conditions are suspicious: the redelivery may not happen not only fort the REJECT reason.
It can be accepted in the error handler or some other way.
Or... Record could be delivered to other consumer on the same share topic.
Therefore we are blocked here for 5 seconds without any clarity.
If there is no way to track rejection somehow on Kafka side, then I don't see any value in this test at all.
Feel like we are trying to test here Kafka client itself: nothing is under our container control.

artembilan · 2026-03-31T18:50:16Z

+		try {
+			assertThat(bothSeenLatch.await(15, TimeUnit.SECONDS)).isTrue();
+			// No redelivery: if a third invocation happened the latch would have reached 0
+			assertThat(redeliveryLatch.await(5, TimeUnit.SECONDS))


I don't think using timing barriers to ensure that something has not happened is the right way to model test behavior.
The event might not happened for may reasons, or our barrier is not long enough to catch that event.

It looks again that you are trying to prove the Kafka client (or even broker) promises with unreliable tools.
I don't know Kafka internal details to give a good advice how to verify that record has been rejected because we had thrown error on it.
But again probably out of our project scope...

artembilan · 2026-03-31T18:57:05Z

+* `ACCEPT` — Record processed successfully; mark as completed.
+* `RELEASE` — Temporary failure; make the record available for redelivery to this or another consumer.
+* `REJECT` — Permanent failure; archive the record and do not redeliver.
+* `RENEW` — Extend the acquisition lock (non-terminal).


Not related to the change in this PR (which is great), but raises the question as natural evolution afterwards.
How to deal with RENEW if we chose EXPLICIT?
Is there a possibility that we have to extend the lock somehow from the container perspective?

hmm - thats a very interesting scenario that I didn't think about. Before this PR, we still had this problem with the old non-"explict" mode where we were not handing an Acknowledgment object to the listener. There we were not allowing listeners to do any renew, which was a true gap and that gap is still present with the new EXPLICIT mode. I guess, we have couple of options - 1. expose a renew-only acknowledgment in explicit mode for listeners to use 2. or document strictly that if they have a renew use case, then they have to use manual mode. We can also auto-renew periodically from the container based on some property maybe. I think we can probably tackle this as a follow-up item maybe?

Issue created to track: #4381

artembilan · 2026-03-31T18:58:59Z


-		// Validate listener type for explicit acknowledgment mode
-		if (containerProperties.isExplicitShareAcknowledgment()) {
+		if (containerProperties.getShareAckMode() == ContainerProperties.ShareAckMode.MANUAL) {


I think we need to use equals() and have it flipped instead of == operator which does not compare objects, but rather their memory pointers:

if (ContainerProperties.ShareAckMode.MANUAL.equals(containerProperties.getShareAckMode())) {

artembilan · 2026-03-31T19:00:18Z

-				// Apply explicit mode configuration to consumer
-				// Note: This should ideally be done during consumer creation in the factory
-				this.logger.info("Share consumer configured for explicit acknowledgment mode");
+			if (shareAckMode == ContainerProperties.ShareAckMode.IMPLICIT) {


artembilan · 2026-03-31T19:00:37Z

+			if (shareAckMode == ContainerProperties.ShareAckMode.IMPLICIT) {
+				ShareConsumerRecordRecoverer recoverer =
+						ShareKafkaMessageListenerContainer.this.getShareConsumerRecordRecoverer();
+				if (recoverer != ShareConsumerRecordRecoverer.REJECTING) {


Here identity is good, but has to be flipped to avoid NPE reports.

artembilan · 2026-03-31T19:03:04Z

+				Object configured = ShareKafkaMessageListenerContainer.this.shareConsumerFactory
+						.getConfigurationProperties()
+						.get(ConsumerConfig.SHARE_ACKNOWLEDGEMENT_MODE_CONFIG);
+				if (configured != null && "implicit".equalsIgnoreCase(configured.toString())) {


Why cannot we use ShareAcknowledgementMode instead of hard-coded string?

artembilan · 2026-03-31T19:03:56Z

+				this.consumer = ShareKafkaMessageListenerContainer.this.shareConsumerFactory.createShareConsumer(
+						ShareKafkaMessageListenerContainer.this.getGroupId(),
+						consumerClientId,
+						Map.of(ConsumerConfig.SHARE_ACKNOWLEDGEMENT_MODE_CONFIG, "explicit"));


DITTO.
See ConsumerConfig defaults:

.define(ConsumerConfig.SHARE_ACKNOWLEDGEMENT_MODE_CONFIG, Type.STRING, ShareAcknowledgementMode.IMPLICIT.name(), new ShareAcknowledgementMode.Validator(), Importance.MEDIUM, ConsumerConfig.SHARE_ACKNOWLEDGEMENT_MODE_DOC)

artembilan · 2026-03-31T19:04:39Z

+							+ "ShareAckMode.IMPLICIT on the container properties instead.");
+				}
+				this.consumer = ShareKafkaMessageListenerContainer.this.shareConsumerFactory.createShareConsumer(
+						ShareKafkaMessageListenerContainer.this.getGroupId(),


I think we use too many ShareKafkaMessageListenerContainer.this.getGroupId() in this method.
Cannot it be extracted to a local variable?

- Use `equals()` with constant first for enum comparisons - Flip identity check for recoverer NPE protection - Use `ShareAcknowledgementMode` enum instead of hardcoded strings - Extract `getGroupId()` to local variable - Remove flaky integration tests that use timing barriers to verify negative conditions Signed-off-by: Soby Chacko <soby.chacko@broadcom.com>

artembilan · 2026-04-01T17:56:21Z

Now that this is merged, @sobychacko , please, take a look if PR #4374 is not a duplication someway, and if this merged PR is fixing respective issue: #4369

Thanks

sobychacko · 2026-04-01T18:06:48Z

Thanks, @artembilan. Will do.

sobychacko added this to the 4.1.0-RC1 milestone Mar 29, 2026

sobychacko added the type: enhancement label Mar 29, 2026

sobychacko mentioned this pull request Mar 29, 2026

GH-4369: Pass explicit acknowledgment mode as consumer override property #4374

Closed

sobychacko force-pushed the share-ack-mode branch from 5027ee2 to aae9b8f Compare March 29, 2026 02:59

artembilan requested changes Mar 30, 2026

View reviewed changes

Address PR review feedback for ShareAckMode

f672fe7

- Simplify whats-new.adoc and move detailed migration guide to ref docs - Remove try-catch around `getConfigurationProperties()` - let it fail if not implemented Signed-off-by: Soby Chacko <soby.chacko@broadcom.com>

artembilan requested changes Mar 31, 2026

View reviewed changes

sobychacko mentioned this pull request Mar 31, 2026

Handling RENEW ack type in EXPLICIT ack mode in Share consumers #4381

Open

artembilan merged commit 813b9af into spring-projects:main Apr 1, 2026
3 checks passed

sobychacko mentioned this pull request Apr 1, 2026

Setting explicit acknowledgment via ShareKafkaListenerContainerFactory container properties does not enable explicit acknowledgment mode for Share Groups #4369

Closed

Conversation

sobychacko commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sobychacko commented Mar 29, 2026

Uh oh!

artembilan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sobychacko Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

artembilan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

artembilan commented Apr 1, 2026

Uh oh!

sobychacko commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sobychacko commented Mar 29, 2026 •

edited

Loading

sobychacko Mar 30, 2026 •

edited

Loading