Under-documented "Manually Assigning All Partitions"

[michaldo]

In what version(s) of Spring for Apache Kafka are you seeing this issue?

3.0.12

Describe the bug
Doc says

it can be useful to manually assign the partitions and not use Kafka’s group management (...) You should also set the container’s AckMode to MANUAL to prevent the container from committing offsets for a null consumer group

That is not enough, because DefaultErrorHandler by default has ackAfterHandle == true and commit the offset on exception.
Should I create and Doc PR to mention

+ You should also set the container’s error handler setAckAfterHandle(false) to prevent the container from committing offsets on error

?

To be honest, preventing the container from committing offsets should work automatically when @TopicPartition is detected...

[garyrussell]

Thanks. The container should detect manual assignment with no group and ignore ackAfterHandle. We can’t assume MANUAL when there is no group. It might be a configuration error.

[michaldo]

Do you mean that

KafkaMessageListenerContainer

private void commitOffsetsIfNeeded(final ConsumerRecords<K, V> records) {
  if ((!this.autoCommit && this.commonErrorHandler.isAckAfterHandle())
	|| this.producer != null) {

should be replaced by

private void commitOffsetsIfNeeded(final ConsumerRecords<K, V> records) {
  if ((!this.autoCommit && this.commonErrorHandler.isAckAfterHandle() && this.containerProperties.getTopicPartitions() != null )
	|| this.producer != null) {

?

[garyrussell]

No, we must only skip the commit if the group is null in that case.

[michaldo]

We can’t assume MANUAL when there is no group. It might be a configuration error.

That sounds inconsistent:

1. Do not commit record on exception according to property ackAfterHandle, because ack does not have a sense, when there is no group.
2. Commit record according to property ackMode, even when there is no group.

If no group + ack is OK, it should be acked always, according to configuration
If no group + ack is NOK, it should never be acked, regardless of configuration

[garyrussell]

That sounds inconsistent:

I agree; we can change the default behavior in the upcoming 3.1 release (due next week(, but not in earlier versions.