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

MessageChannelBinder support for futures #2091

Open
GreenRover opened this issue Jan 21, 2021 · 3 comments
Open

MessageChannelBinder support for futures #2091

GreenRover opened this issue Jan 21, 2021 · 3 comments

Comments

@GreenRover
Copy link

GreenRover commented Jan 21, 2021

Following is related to: SolaceProducts/solace-spring-cloud#35

I want to do some brain storming how to solve this problematic most elegant.

The Problem:

  • When sending a message to broker
  • The response from broker if the messages was accepted gets async
  • This leads currently to that not submitted messages will only be reported on error channel
  • But this makes it very hard for the application to handle those problem, caused by the missing relation.

Possible improvement A:

https://github.com/spring-projects/spring-integration/blob/a66e82b0aad0eb3ffb909dd5568c14b6713259c0/spring-integration-core/src/main/java/org/springframework/integration/channel/AbstractSubscribableChannel.java#L70

@Override
protected boolean doSend(Message<?> message, long timeout) {
    try {
        CompletableFuture<MessaggingExecption> asyncErrorConsumer = new CompletableFuture<>();
        message.getHeaders().add(MessageHeaders.ERROR_CONSUMER, asyncErrorConsumer);
        
        boolean result = getRequiredDispatcher().dispatch(message);
        
        if (!result) {
          return result;
        }  
        
        MessagingException possibleError = asyncErrorConsumer.get(timeout, TimeUnit.SECONDS);
        if (possibleError != null) {
          throw possibleError;
        }
    
        return result;
    }
    catch (MessageDispatchingException ex) {
        String description = ex.getMessage() + " for channel '" + getFullChannelName() + "'.";
        throw new MessageDeliveryException(message, description, ex);
    }
}
  • The code is everything not nice
  • It is a breaking change that all binders have to implement
  • The benefit of async message persistant confirmation is gone for all producer wanting fire and forget

Possible improvement B:

Rewrite all the sender code including StreamBridge to not return "boolean" but instead "Future"

  • Big refactoring in spring-cloud-stream and spring-integration-core
  • Breaking change for all binder
  • Allows applications to make fire and forget as well as wait for confirmation that the message was persisted/accepted

Possible improvement C:

An äquivalent to:
https://docs.spring.io/spring-cloud-stream-binder-rabbit/docs/3.1.0/reference/html/spring-cloud-stream-binder-rabbit.html#publisher-confirms

But i opened this issue to find a better solution that also could be used from rabbit.

  • A developer can choose if he wants a confirmation.
  • Its not very straight forward. (Not easy to understand)
  • No changes on spring framework.
@garyrussell
Copy link
Contributor

Not a general solution, I know, but there is a solution for the RabbitMQ binder that uses a similar technique to A...

https://docs.spring.io/spring-cloud-stream-binder-rabbit/docs/3.1.0/reference/html/spring-cloud-stream-binder-rabbit.html#publisher-confirms

@GreenRover
Copy link
Author

@garyrussell i know, that is what i checked first. But this solution ist not very straight forward. If i as developer should use thinks like this, i dont know if i like it. But i will add it to the possibility options's.

@UgiR
Copy link

UgiR commented Feb 12, 2021

+1 for this kind of feature. Would also be a plus for this to expose a CompletableFuture, so it can be easy to use with reactive: Mono.fromFuture(completableFuture)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants