Commit Graph

85 Commits

Author SHA1 Message Date
Artem Bilan
8a3e562f43 GH-154: Copy ContainerProperties in the Container
Fixes: GH-154 (https://github.com/spring-projects/spring-kafka/issues/154)

Previously the `AbstractMessageListenerContainer` save external `ContainerProperties` instance leaving the room for external/internal mutation,
which may lead to unexpected behavior

* Create a new instance of the `ContainerProperties` in the `AbstractMessageListenerContainer` ctor based on the provided `ContainerProperties`
and use `BeanUtils.copyProperties()` for convenience.
* Refactoring for tests to meet a new state. Some of them indeed modified `ContainerProperties` after container instantiation.
* Remove fake `"propertiesFactory"` topic from the `AbstractKafkaListenerContainerFactory.containerProperties` instance to avoid unexpected behavior.
Change that to `(Pattern) null`

**Cherry-pick to 1.0.x**

Conflicts:
	spring-kafka/src/test/java/org/springframework/kafka/listener/ConcurrentMessageListenerContainerTests.java
Resolved.
2016-07-28 15:39:32 -04:00
Artem Bilan
c703a30714 GH-150: Don't eat errors in AbstractAdaptableML
Fixes: GH-150 (https://github.com/spring-projects/spring-kafka/issues/150)

In some corner cases the target `MessageListener` implementation may decide to invoke `AbstractAdaptableMessageListener.onMessage(message)` of its delegate.
In this case we can't receive any exceptions back into container for handling, because `handleListenerException()` just logs them.

* Deprecate `handleListenerException()` and don't wrap `onMessage(message, null)` call with `try...catch` to let exception bubble in container back,
like it is with regular `onMessage(message, null)` in the container.

(Consider to remove `handleListenerException()` altogether for current `1.1` `master`)

**Cherry-pick to 1.0.x**

De-`@Deprecate` `handleListenerException()`

A custom implementation may decide to use that method for its purpose.
2016-07-19 18:40:51 -04:00
Spring Buildmaster
b37277d89c [artifactory-release] Next development version 2016-07-12 18:35:11 +00:00
Spring Buildmaster
9ec3a78015 [artifactory-release] Release version 1.0.2.RELEASE 2016-07-12 18:35:08 +00:00
Gary Russell
32fcf90c36 GH-146: Fix TopicPartition Negative Reset
Fixes #146

When using reset relative to the current end, ensure the seek value is
not less than zero.

Add `consumer.position()` to error log message to reflect reality on `seek()`failure
(cherry picked from commit 6d80e0e)
2016-07-12 13:47:18 -04:00
Artem Bilan
a353de3877 GH-140: Commit in the Beginning of the Main Loop
Fixes GH-140 (https://github.com/spring-projects/spring-kafka/issues/140)

Previously with the pretty big `containerProperties.getPollTimeout()` we ended up with the issue of not acked commits for `BATCH` mode.
Just because the logic relies on the `consumer.wakeup()` which causes `WakeupException` breaking the main polling loop.

* Move `processCommits()` function to the beginning of main loop, before blocking on the `this.consumer.poll()`
* Increase `EnableKafkaIntegrationTests` wait timeouts

**Cherry-pick to 1.0.x**

Add `firstBatchLatch` to test to verify that the first batch is commit within the time less than 10 sec for poll
2016-07-12 13:36:41 +01:00
Gary Russell
3d79310672 CLA Hook 2016-07-12 13:36:31 +01:00
Artem Bilan
819a0ce844 GH-141: Add MANUAL acks only in acknowledge()
Fixes GH-141 (https://github.com/spring-projects/spring-kafka/issues/141)

The `MANUAL` acks are intended to be acknowledged only by the end-listener initiative.
An unconditional `this.acks.add(record)` is in the `invokeListener()` function by mistake.

* Wrap `this.acks.add(record)` in the `invokeListener()` to `if (!this.isAnyManualAck)`
* Fix `NPE` in te main poll loop as `if (records != null && this.logger.isDebugEnabled())`
(cherry picked from commit 44cc15f)
2016-07-07 20:45:26 -04:00
Spring Buildmaster
840a62e257 [artifactory-release] Next development version 2016-07-05 21:54:55 +00:00
Spring Buildmaster
b0b39fd951 [artifactory-release] Release version 1.0.1.RELEASE 2016-07-05 21:54:52 +00:00
Igor Stepanov
f08b916d28 GH-134: AssertJ style for thrown assertions
**Cherry-pick to 1.0.x**
(cherry picked from commit aa4db96)
2016-07-05 17:31:49 -04:00
Igor Stepanov
c193b2c1b0 GH-134: Assert KafkaTemplate.flush() for NPE
Fixes GH-134 (https://github.com/spring-projects/spring-kafka/pull/137)
(cherry picked from commit 6a94ae2)
2016-07-05 17:21:30 -04:00
Gary Russell
fa56160b94 GH-135: Fix Acks
Resolves #135

Properly transfer `ack`s from the listener thread to the consumer thread
for all ack modes.

Previously, record ack mode was not handled in `processCommits`.

Fix test according PR comments
(cherry picked from commit 677d135)
2016-07-05 12:01:40 -04:00
Artem Bilan
e43058a14a Branch 1.0.x 2016-07-01 14:17:56 -04:00
Spring Buildmaster
cce7ae9dc5 [artifactory-release] Next development version 2016-07-01 17:20:37 +00:00
Spring Buildmaster
89d7940f18 [artifactory-release] Release version 1.0.0.RELEASE 2016-07-01 17:20:32 +00:00
Artem Bilan
0b3c26b551 Upgrade some dependencies 2016-07-01 13:02:24 -04:00
Gary Russell
51af726693 GH-118: Rework Manual AckMode
Fixes #118

- Remove MANUAL_IMMEDIATE_SYNC - sync/async is controlled by the `syncCommits` property
- For MANUAL, wake the consumer thread at the end of the batch
- For MANUAL_IMMEDIATE, the consumer thread is woken directly from the Acknowledgement

Polishing

* Some simple code reformatting for `KafkaMessageListenerContainer`
* Move the `CountDownLatch` logic in the `KafkaMessageListenerContainerTests#testSlowConsumerCommitsAreProcessed()`
into the mock for `Consumer.commitSync()` since the count logic in the listener immediately after `ack.acknowledge()` doesn't guaranty that
`Consumer.commitSync()`will be called. That is because listener lives in one Thread, but `processCommits()` is done from a different `ListenerConsumer` Thread
2016-06-30 15:37:47 -04:00
Artem Bilan
3483387eb0 GH-124: Validate ackCount and ackTime
Fixes GH-124 (https://github.com/spring-projects/spring-kafka/issues/124)

That doesn't look reasonable to have void `commitIfNecessary()` if we exceed default `0` for `ackCount` or `ackTime`.

* Add requirement to have `ackCount` and `ackTime` `> 0` in case appropriate `ackMode`
* Make `ackTime` as 5 secs by default - similar to default for `auto.commit.interval.ms`

Address PR comments

* Don't require `count` in case of `BATCH` mode
* Make `ackTime` as 5 sec only when `**TIME` mode
* Validate provided `count` and `time` options in the `ContainerProperties`
* Mention in JavaDocs for `MANUAL` that it works as `MANUAL_IMMEDIATE_SYNC` when no `count` and `time`
* Restore changes for tests
* Overcome the `Assert`s with `if` when `BeanUtils.copyProperties` is used
2016-06-28 11:23:26 -04:00
Marius Bogoevici
be2e6ce984 GH-117: Move the placement of manual ack handling
Fixes GH-117 (https://github.com/spring-projects/spring-kafka/issues/117)

- merge `handleManualAcks`
- simplify offset handling by removing the `manualOffsets` map
- ensure that all acks are flushed on `stop()`
- Upgrade to Gradle 2.14
2016-06-20 12:11:07 -04:00
Artem Bilan
0e164d8678 Fix race condition in the KafkaMessageLCTests
Having the async consumption nature we have to `spy` the `Consumer` before the real consumption.
Otherwise we can end up with the fact that we pass all desired phases in one thread, but another hasn't been spied yet to be verified in the end properly.

https://build.spring.io/browse/SK-SON-97/
2016-06-16 11:32:46 -04:00
Gary Russell
df23749a60 GH-115: S-I-K Documentation
Resolves #115

Also fix some minor PDF formatting issues.
Fix typo.
2016-06-07 19:34:18 -04:00
Spring Buildmaster
5b03bd2841 [artifactory-release] Next development version 2016-06-07 00:17:35 +00:00
Spring Buildmaster
39e8552b49 [artifactory-release] Release version 1.0.0.RC1 2016-06-07 00:17:33 +00:00
Artem Bilan
5296d561b4 Some minor upgrades 2016-06-06 19:55:57 -04:00
Gary Russell
e5a3c6f459 .gitignore Eclipse .checkstyle Files 2016-06-04 13:36:02 -04:00
Martin Dam
0fb829b3ad GH-112: Commit offsets even when fetch is paused
Fixes GH-112 (https://github.com/spring-projects/spring-kafka/issues/112)

Increase timeout on unrelated failing tests

* Polishing according PR comments
2016-06-03 19:05:12 -04:00
Artem Bilan
06616f73ed GH-89: Add Support for Topic Seek
Fixes GH-89 (https://github.com/spring-projects/spring-kafka/issues/89)

* Introduce `TopicPartitionInitialOffset`, where it utilizes `TopicPartition` and `Long initialOffset`
* The `initialOffset` can be:
  - `null` - do nothing;
  - positive (including `0`) - absolute offset
  - negative - the offset relative to the current last offset of the partition: `consumer.seekToEnd() + initialOffset`
* Rework everything around to rely on a new `TopicPartitionInitialOffset` abstraction
* The logic in the `KafkaMessageListenerContainer.ListenerConsumer.initPartitionsIfNeeded()` reworked to in favor of a new abstraction
* remove redundant `recentOffset`
* Reflect new `TopicPartitionInitialOffset` in the docs

Add `@PartitionOffset` support for the `@TopicPartition`

Polishing
2016-06-03 12:02:31 -04:00
Marius Bogoevici
67ffc2e331 GH-108: Stop the Invoker in the Middle of a Batch
- Fixes #110, committing the initial state of a consumer

Polishing according PR comments

Don't Interrupt invoker unless he doesn't stop

Logging and Clear Unsent
2016-06-02 14:59:17 -04:00
Gary Russell
e7240e207d GH-105: Externalize Retry
Resolves #105

Extract the retry template into adapters.

Polishing

Redo `ConcurrentMessageListenerContainerTests` to Java 8 style.
Change `ArrayList` for thread names to the `ConcurrentSkipListSet` which is synchronized
2016-06-02 12:23:23 -04:00
Marius Bogoevici
be8b438fdd GH-87: Add ackOnError and fix ack commit issues
Fixes #87 (https://github.com/spring-projects/spring-kafka/issues/87)

- allow for acking on both success and error (make the latter conditional upon `ackOnError`)
- fixes an issue where `processCommits()` wasn't handled on invoker stop, allowing for missed commits
2016-06-02 11:02:18 -04:00
Gary Russell
b1dc76e3a9 GH-101: Rename DeDup Classes/Interfaces to Filter
Resolves #101

Polishing - ackDiscarded on @KafkaListener
2016-06-01 16:48:49 -04:00
Gary Russell
eff22d0922 GH-99 Idle Container Events
Resolves #99

Also change `ContainerProperties` to use accessors

Also fix `stop(Runnable callback` logic for the container registry and
concurrent container.

* Polishing according PR comments and some typos fixes
2016-06-01 13:38:43 -04:00
Gary Russell
31541fe58e GH-84: Pause/Resume
Resolves #84

Slow consumers can cause a rebalance when not required.

Solution: 2 threads per consumer
 - beanName-kafka-consumer-n
 - beanName-kafka-listener-n

The first constantly polls Kafka; for each retrieved message then hands off
the message to the second thread (via a `ListenableFuture`).
If the future completes ok (`get()`, all is well and we move on to the next
message, if not, the consumer is paused and we continue to poll (heartbeat).

When the callback completes successfully, the consumer is awoken from the poll
and the next previously retrieved message is processed. When all such messages
are processed, the consumer is resumed.

If manual acks are in play, the ack is passed from the listener thread back to
the consumer thread via a `BlockingQueue` so the actual commit is done on the
right thread.

If the delayed execution fails, normally, the error is reported to the error
handler, the message is ack'd and we move on to the next as with a successful
completion.

However, to satisfy the use case reported by issue 84, if the exception is
assignable to the configured `pausedException` class, the consumer is
paused as with a timeout. In this case, after the next poll, the same
record is attempted to be delivered. Deliveries continue until successful
or some other exception is thrown.

Properties:

- `pauseWhenSlow` - enable pause/resume (default `true`)
- `pauseAfter` - how long to wait for the listener thread to return
- `pauseException` - an exception that will cause a pause

Polishing

Polishing - Use a BlockingQueue

In order to maintain a pipeline of work for the listener invoker,
hand ConsumerRecords from each poll to the listener via a blocking
queue.

Now, pause the consumer only if the blocking queue becomes full.

The queue size is configurable (default 1) so we pause when 2
ConsumerRecords have been retrieved.

Remove the `pauseForException` code - instead use a `RetryTemplate` in
the listener invoker configured to retry for an exception of choice.

Since retry generally uses a backoff - this will put backpressure on the
queue and the consumer will be paused.

There's a slight semantic change to `AckMode.BATCH` in that it now means
acks for currently processed records will be committed after each poll;
it may not be the complete batch.

Suspend if not pauseEnabled

Enhacements to GH-84

- guarantee that the container is effectively stopped when stop() returns;
- On topic and topic pattern subscriptions, clear pending messages and
flush acknowledgments;
- On topic and topic pattern subscriptions, use `partitionsAssigned` as trigger
for starting the invoker

Additional enhancements to GH-84

- Run autocommit containers on a single thread

Additional enhancements to GH-84

- only start the invoker if the container is running and we have partitions to listen

Additional changes to GH-84

Use `cancel` to stop the listener and interrupt the poll
Allow for asynchronous stop in `stop(Runnable)`
Simplify classes used by the tests

Reinstate isPauseEnabled Use

Interrupt thread explicitly and wait for a clean stop on the invoker

Create ContainerProperties

Move ContainerProperties to Ctor Argument

Add topic config to the properties.

Since the annotated listener is created by a factory, we have
to "clone" the properties.

Doc Polishing for ContainerProperties

Move ContainerProperties to a Top Level Class

Simple polishing
2016-06-01 11:44:32 -04:00
Jérôme Mirc
903ef26622 Propagate ErrorHandler to KMLC
When a KafkaMessageListenerContainer is created, the error handler method is not called. There is no way to set the error handler after it is created

Fix codestyle

Update the testListenerException test case to test the errorHandler method

Add my name as requested.

Remove accent on my name
2016-05-31 10:03:24 -04:00
Gary Russell
99d31807bd GH-95: Fix @TopicPartition Array Resolution
Fixes: #95
2016-05-27 13:52:58 -04:00
Gary Russell
be016f0f7d GH-93: Resolve Placeholder in @TopicPartition
Fixes #93

Polishing for test config
2016-05-27 13:02:33 -04:00
Igor Stepanov
6a0813a534 GH-88: Stop using slf4j
Fixes GH-88 (https://github.com/spring-projects/spring-kafka/issues/88)

 - unused logger removed
 - inherited dependencies excluded

* Dependencies polishing
2016-05-18 14:54:47 -04:00
Artem Bilan
79250ff7a6 Polishing according Spring Code style
* Remove `Lombok` dependency
* Allow to write tests with Java 8
* Remove redundant `JsonDatabindFactory` in favor of `ObjectMapper` injection into the `JsonSerializer`/`JsonDeserializer`
* Remove `JsonWrapperException` in favor of existing `SerializationException` in Apache Kafka
* Fix `StringJsonMessageConverter` to let inject `ObjectMapper`
* Fix inline `ObjectMapper` for the
```
this.objectMapper.configure(MapperFeature.DEFAULT_VIEW_INCLUSION, false);
this.objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
```
to align with all other Spring projects
* Fix `JsonSerializationTests` for the Spring Code style
* Document Serialization and Conversion components
2016-05-17 14:18:08 -04:00
Igor Stepanov
db17d6620a GH-79: Simplifies POJO-to-JSON serialization and reverse deserialization
- implementation is based on "Serializer" and "Deserializer" concepts from 'kafka-clients' module
 - updated Jackson dependency to 2.6.5 (same is used in Spring Boot v.1.3.3.RELEASE)
 - migrated the related unit tests

GH-79: Use the already implemented Spring utility class instead of native java approach

GH-79: Code formatting according to Spring Conventions, checkstyle fixes, code polishing
2016-05-17 14:17:00 -04:00
Gary Russell
5fa753cbb9 GH-80: Add DeDuplication Listener Adapters
Resolves #80

CheckStyle Fixes

Polishing - PR Comments

Upgrade to Gradle 2.13
2016-05-16 18:31:08 -04:00
Gary Russell
da88336b83 GH-76: Resolve Template Ambiguity with String Keys
Resolves #76

Polishing

Introduce sendDefault() methods instead.

More Polishing
2016-05-13 14:24:39 -04:00
Gary Russell
560ff6f088 Rename SimpleKafkaListenerContainerFactory
Rename to `ConcurrentConcurrentKafkaListenerContainerFactory` to reflect
the type of container it actually creates.
2016-05-13 13:07:07 -04:00
Gary Russell
ead9783087 GH-75: ContainerFactory Improvements
Resolves #75

Add `syncCommit` and `commitCallback` to container factory.
2016-05-13 12:04:40 -04:00
Gary Russell
d93d91b339 GH-62; commitSync() By Default
Fixes #62
Resolves #72

See the discussion on GH-62 `commitAsync()` is not currenly reliable.
Use `commitSync()` by default; add `syncCommits` property to the containers
(default true).

Also allow a user-injected commit callback (GH-72)
2016-05-12 15:56:15 -04:00
Gary Russell
964beb67e2 GH-71: Remove GS Collections from -test
Resolves #71
2016-05-12 13:48:21 -04:00
Artem Bilan
e1174e09f3 Remove duplicated test since the last rebase 2016-05-11 17:25:16 -04:00
Artem Bilan
48845e1de0 GH-61: Add (De)Serializer options for Factories
Fixes GH-61 (https://github.com/spring-projects/spring-kafka/issues/61)

Added setter injections for both (De)Serializers in Consumer and Producer
2016-05-11 17:13:32 -04:00
Murali Reddy
e857c248eb GH-66: Add ConsumerRebalanceListener injection
Fixes GH-66 (https://github.com/spring-projects/spring-kafka/issues/66)

Added fixes for code review comments by artembilan

Added minor documentation change.

Added injection of RebalanceListener to SimpleKafkaListenerContainerFactory

Polishing
2016-05-10 13:25:27 -04:00
Gary Russell
283ca09650 GH-63: Add AckMode.MANUAL_IMMEDIATE_SYNC
Resolves #63
Fixes #58

Add an option for synchronous acks.
2016-05-09 12:25:15 -04:00