Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asynchronous server-side processing in a request/reply scenario #1189

Closed
dnitzan opened this issue Aug 7, 2019 · 12 comments · Fixed by #2996
Closed

Asynchronous server-side processing in a request/reply scenario #1189

dnitzan opened this issue Aug 7, 2019 · 12 comments · Fixed by #2996

Comments

@dnitzan
Copy link

dnitzan commented Aug 7, 2019

Request/reply semantics requires that the server side listener method (annotated with @SendTo) return a response synchronously. There are use cases where server side request processing is asynchronous such that the listener method can't return a result immediately and must defer the response, releasing the consumer thread to handle other incoming requests. Once the result is available, an application thread would need to send it back to the client. This would be a feature similar to Servlet 3.0 Asynchronous Processing.

@garyrussell
Copy link
Contributor

We already added this for @RabbitListener last year.

However, managing the offsets for Kafka could make this tricky to implement - we wouldn't want to commit the offset until the async operation is complete.

If the next async operation completes first, and its offset committed, and then the first one fails, it's too late because its offset has already been implicitly committed.

Since Kafka message are not discretely acknowledged, going async with Kafka record consumption is really not recommended because of the risk of message loss and/or the complexity of managing offsets.

For this reason we didn't proceed here, but we can put it on the backlog and give it some more thought.

@dnitzan
Copy link
Author

dnitzan commented Aug 7, 2019

True, we faced the same issues building a custom request/reply mechanism on top of Spring Kafka. We eventually elected to just commit periodically (with ack-mode=time) regardless of downstream asynchronous processing and it is perfectly fine for us. If such a relaxed guarantee is sufficient, then it greatly simplifies the implementation.

@garyrussell
Copy link
Contributor

garyrussell commented Aug 7, 2019

OK; with that caveat; we'll try to get it into a future release.

@dcheung2
Copy link

I think it is reasonable difficulty. But still may could be done even without a full prefect asynchronized flow.

for example.

when ackMode=BATCH
-> That container could async all the listeners within same BATCH but refer the ack until all messages in a poll() responsed

when ackMode=TIME
-> same as BATCH, but synchronize all once by time, not poll()

when ackMode=RECORD
-> No real concurrently processing. Just one message a time, but allow developers to use async API.

The real difficulty is timeout/retry management which make the state complex.

For another example,
if the first message jammed, the 2nd failed, but 3rd successes. should it ack or retry from where ?
but this is a problem business logic to worry about. not the middleware
That spring-kafka should provide some configuration.

garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Jul 7, 2021
Preparation for spring-projects#1189

Defer committing out of order offsets until the gaps are filled.

Pause the consumer until all acks are acknowledged.
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Jul 7, 2021
Preparation for spring-projects#1189

Defer committing out of order offsets until the gaps are filled.

Pause the consumer until all acks are acknowledged.
artembilan pushed a commit that referenced this issue Jul 7, 2021
Preparation for #1189

Defer committing out of order offsets until the gaps are filled.

Pause the consumer until all acks are acknowledged.
@garyrussell
Copy link
Contributor

I have added support for out-of-order manual commits, which will now make implementing this possible.

garyrussell added a commit that referenced this issue Sep 3, 2021
Add note about increased duplicates possibility.
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Sep 13, 2021
When pending acks are released due to a gap being "filled", wake the consumer
if it is currently paused.
artembilan pushed a commit that referenced this issue Sep 13, 2021
When pending acks are released due to a gap being "filled", wake the consumer
if it is currently paused.
@garyrussell garyrussell modified the milestones: Backlog, 3.1 Backlog May 24, 2023
@garyrussell
Copy link
Contributor

Also support Kotlin Coroutines, discussed here #2653

@Wzy19930507
Copy link
Contributor

Hi, @sobychacko @artembilan
may i pick it up.

@sobychacko
Copy link
Contributor

@Wzy19930507 Certainly!

@artembilan
Copy link
Member

I'm sorry. What is the plan for this issue?
Looks like Gary has added already enough functionality.
What else you see has to be fixed?

Thanks

@garyrussell
Copy link
Contributor

What else you see has to be fixed?

Add support for Mono and Future listener method return types in the MMLA, similar to what we have in RabbitMQ.

This is now possible when out of order commits are enabled.

@artembilan
Copy link
Member

Cool!
Then it sounds like a copy/paste from the: https://github.com/spring-projects/spring-amqp/blob/main/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/adapter/AbstractAdaptableMessageListener.java#L375

Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Dec 21, 2023
* Support `Mono` and `Future`
* Support auto ack at async return scenario when manual commit
* Support `KafkaListenerErrorHandler`
* Add warn log if the container is not configured for out-of-order manual commit
* Add async return test in `BatchMessagingMessageListenerAdapterTests`
  and `MessagingMessageListenerAdapterTests`
* Add unit test async listener with `@SendTo` in `AsyncListenerTests`
* Add `async-returns.adoc` and `whats-new.adoc`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Dec 21, 2023
* Support kotlin suspend
* Add kotlin suspend test, async request/reply with `@KafkaListener` `@KafkaHandler` and `@SendTo`
* Fix async-returns.adoc and async warn log in `MessagingMessageListenerAdapter`
* Add dependency `org.jetbrains.kotlinx:kotlinx-coroutines-reactor:1.7.3`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Dec 28, 2023
* auto-detect async reply than coerce the out-of-order manual commit.
* add new interface `HandlerMethodDetect` to detect handler args and return type.
* add auto-detect async reply than coerce the out-of-order manual commit unit test at `@KafkaListener` `@KafkaHandler` scene.
* modify async-returns.adoc
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 14, 2024
…handled

Sending the result from a `KafkaListenerErrorHandler` was broken
for `@KafkaHandler` because the send to expression
was lost.
@artembilan artembilan modified the milestones: 3.1 Backlog, 3.2.0-M1 Jan 16, 2024
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 18, 2024
* Support `Mono` and `Future`
* Support auto ack at async return scenario when manual commit
* Support `KafkaListenerErrorHandler`
* Add warn log if the container is not configured for out-of-order manual commit
* Add async return test in `BatchMessagingMessageListenerAdapterTests`
  and `MessagingMessageListenerAdapterTests`
* Add unit test async listener with `@SendTo` in `AsyncListenerTests`
* Add `async-returns.adoc` and `whats-new.adoc`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 18, 2024
* Support kotlin suspend
* Add kotlin suspend test, async request/reply with `@KafkaListener` `@KafkaHandler` and `@SendTo`
* Fix async-returns.adoc and async warn log in `MessagingMessageListenerAdapter`
* Add dependency `org.jetbrains.kotlinx:kotlinx-coroutines-reactor:1.7.3`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 18, 2024
* auto-detect async reply than coerce the out-of-order manual commit.
* add new interface `HandlerMethodDetect` to detect handler args and return type.
* add auto-detect async reply than coerce the out-of-order manual commit unit test at `@KafkaListener` `@KafkaHandler` scene.
* modify async-returns.adoc
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 18, 2024
…handled

Sending the result from a `KafkaListenerErrorHandler` was broken
for `@KafkaHandler` because the send to expression
was lost.
Wzy19930507 added a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
* Support `Mono` and `Future`
* Support auto ack at async return scenario when manual commit
* Support `KafkaListenerErrorHandler`
* Add warn log if the container is not configured for out-of-order manual commit
* Add async return test in `BatchMessagingMessageListenerAdapterTests`
  and `MessagingMessageListenerAdapterTests`
* Add unit test async listener with `@SendTo` in `AsyncListenerTests`
* Add `async-returns.adoc` and `whats-new.adoc`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
* Support kotlin suspend
* Add kotlin suspend test, async request/reply with `@KafkaListener` `@KafkaHandler` and `@SendTo`
* Fix async-returns.adoc and async warn log in `MessagingMessageListenerAdapter`
* Add dependency `org.jetbrains.kotlinx:kotlinx-coroutines-reactor:1.7.3`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
* auto-detect async reply than coerce the out-of-order manual commit.
* add new interface `HandlerMethodDetect` to detect handler args and return type.
* add auto-detect async reply than coerce the out-of-order manual commit unit test at `@KafkaListener` `@KafkaHandler` scene.
* modify async-returns.adoc
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
…handled

Sending the result from a `KafkaListenerErrorHandler` was broken
for `@KafkaHandler` because the send to expression
was lost.
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
* Support `Mono` and `Future`
* Support auto ack at async return scenario when manual commit
* Support `KafkaListenerErrorHandler`
* Add warn log if the container is not configured for out-of-order manual commit
* Add async return test in `BatchMessagingMessageListenerAdapterTests`
  and `MessagingMessageListenerAdapterTests`
* Add unit test async listener with `@SendTo` in `AsyncListenerTests`
* Add `async-returns.adoc` and `whats-new.adoc`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
* Support kotlin suspend
* Add kotlin suspend test, async request/reply with `@KafkaListener` `@KafkaHandler` and `@SendTo`
* Fix async-returns.adoc and async warn log in `MessagingMessageListenerAdapter`
* Add dependency `org.jetbrains.kotlinx:kotlinx-coroutines-reactor:1.7.3`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
* auto-detect async reply than coerce the out-of-order manual commit.
* add new interface `HandlerMethodDetect` to detect handler args and return type.
* add auto-detect async reply than coerce the out-of-order manual commit unit test at `@KafkaListener` `@KafkaHandler` scene.
* modify async-returns.adoc
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 19, 2024
…handled

Sending the result from a `KafkaListenerErrorHandler` was broken
for `@KafkaHandler` because the send to expression
was lost.
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 30, 2024
* Support `Mono` and `Future`
* Support auto ack at async return scenario when manual commit
* Support `KafkaListenerErrorHandler`
* Add warn log if the container is not configured for out-of-order manual commit
* Add async return test in `BatchMessagingMessageListenerAdapterTests`
  and `MessagingMessageListenerAdapterTests`
* Add unit test async listener with `@SendTo` in `AsyncListenerTests`
* Add `async-returns.adoc` and `whats-new.adoc`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 30, 2024
* Support kotlin suspend
* Add kotlin suspend test, async request/reply with `@KafkaListener` `@KafkaHandler` and `@SendTo`
* Fix async-returns.adoc and async warn log in `MessagingMessageListenerAdapter`
* Add dependency `org.jetbrains.kotlinx:kotlinx-coroutines-reactor:1.7.3`
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 30, 2024
* auto-detect async reply than coerce the out-of-order manual commit.
* add new interface `HandlerMethodDetect` to detect handler args and return type.
* add auto-detect async reply than coerce the out-of-order manual commit unit test at `@KafkaListener` `@KafkaHandler` scene.
* modify async-returns.adoc
Wzy19930507 pushed a commit to Wzy19930507/spring-kafka that referenced this issue Jan 30, 2024
…handled

Sending the result from a `KafkaListenerErrorHandler` was broken
for `@KafkaHandler` because the send to expression
was lost.
Wzy19930507 added a commit to Wzy19930507/spring-kafka that referenced this issue Jan 30, 2024
…ing-projectsGH-1189

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/listener/adapter/DelegatingInvocableHandler.java
#	spring-kafka/src/main/java/org/springframework/kafka/listener/adapter/HandlerAdapter.java
#	spring-kafka/src/main/java/org/springframework/kafka/listener/adapter/MessagingMessageListenerAdapter.java
artembilan pushed a commit that referenced this issue Jan 31, 2024
Fixes: #1189

* Refactor `MessagingMessageListenerAdapter`
* move `BatchMessagingMessageListenerAdapter#invoke` and `RecordMessagingMessageListenerAdapter#invoke` to `MessagingMessageListenerAdapter`
* move `KafkaListenerErrorHandler` to `MessagingMessageListenerAdapter`
* add `@Nullable` to `KafkaListenerErrorHandler`
* GH-1189: support `Mono` and `Future`
* Support auto ack at async return scenario when manual commit
* Support `KafkaListenerErrorHandler`
* Add warn log if the container is not configured for out-of-order manual commit
* Add async return test in `BatchMessagingMessageListenerAdapterTests`
  and `MessagingMessageListenerAdapterTests`
* Add unit test async listener with `@SendTo` in `AsyncListenerTests`
* Add `async-returns.adoc` and `whats-new.adoc`
* GH-1189: support Kotlin `suspend`
* Add kotlin suspend test, async request/reply with `@KafkaListener` `@KafkaHandler` and `@SendTo`
* Fix async-returns.adoc and async warn log in `MessagingMessageListenerAdapter`
* Add dependency `org.jetbrains.kotlinx:kotlinx-coroutines-reactor:1.7.3`
* auto-detect async reply than coerce the out-of-order manual commit.
* add new interface `HandlerMethodDetect` to detect handler args and return type.
* add auto-detect async reply than coerce the out-of-order manual commit unit test at `@KafkaListener` `@KafkaHandler` scene.
* modify async-returns.adoc
* GH-1189: `@SendTo` for `@KafkaHandler` after error is handled
* Sending the result from a `KafkaListenerErrorHandler` was broken
  for `@KafkaHandler` because the send to expression was lost.
* add javadoc in `AdapterUtils`
* move class from package `annotation` to package `adapter`
* re name bar,baz in BatchMessagingMessageListenerAdapterTests
* poblish unit test `MessagingMessageListenerAdapterTests` and `EnableKafkaKotlinCoroutinesTests`
* poblish doc async-returns.adoc and nav.adoc
* rename `HandlerMethodDetect` to `AsyncRepliesAware`
* fix javadoc in `ContinuationHandlerMethodArgumentResolver`
* After kafka client 2.4 producer uses sticky partition, its randomly chose partition and topic default partitions is 2, configure that `@EmbeddedKafka `to provide just one partition per topic.
* javadoc in `AsyncRepliesAware`
* fix test in EnableKafkaKotlinCoroutinesTests
* polish adoc
* polish `DelegatingInvocableHandler` and add javadoc
* polish `HandlerAdapter`
* change `InvocationResult` to record
* Optimization `MessagingMessageListenerAdapter.asyncFailure`
* Mention version in the doc for async return types
artembilan added a commit that referenced this issue Feb 12, 2024
Related to #1189

* Some other code clean up including deprecation warning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment