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

Add more producer error handling options #35

Open
Nephery opened this issue Nov 17, 2020 · 9 comments
Open

Add more producer error handling options #35

Nephery opened this issue Nov 17, 2020 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@Nephery
Copy link
Collaborator

Nephery commented Nov 17, 2020

Today the binder only sends publisher errors to error channels. We should consider adding other error handling e.g. maybe adding error queue support?

#34 (comment)
Also I think we need specific error handling depending on the JCSMPErrorResponseException. For example if the message is too large then we won't be able to republish it to the error queue anyway, but if it was invalid topic syntax sending to an error queue might make more sense. There are other issues such as the MAX_MESSAGE_USAGE_EXCEEDED and SPOOL_OVER_QUOTA that a retry could make sense as space may have been freed up by consumers.

image


Note: October 21, 2021

Ignore all comments from #35 (comment) to #35 (comment). These comments are irrelevant to this issue.

@Nephery Nephery added the enhancement New feature or request label Nov 17, 2020
@GreenRover
Copy link
Contributor

I currently ran into same situation.
The solution of the errorChannel is not very elegant.

But i have seen that this is more a spring cloud stream bug caused by:

com.solace.spring.cloud.stream.binder.outbound.JCSMPOutboundMessageHandler.handleMessage
called by:
org.springframework.cloud.stream.binder.AbstractMessageChannelBinder.handleMessageInternal
is completely wrong designed.

Or more specific, the whole async sending is not a god plan!
org.springframework.integration.channel.AbstractSubscribableChannel.doSend

This caused me to do the sending by my own:
Using:

@Autowired
private final BinderFactory binderFactory;

SolaceMessageChannelBinder session = binderFactory.getBinder("main_session", PublishSubscribeChannel.class);

But here i stuck again, because it is not possible to access the jcsmpSession.
And the JCSMPOutboundMessageHandler supports only async sending.

@Nephery do you have any plans to add synchronous sending? Or even better do you already any architecture designs?

@Mrc0113
Copy link
Contributor

Mrc0113 commented Dec 17, 2020

@GreenRover - completely agree that the current solution is not very elegant. That said, I don't think we want to move to a synchronous send as it would kill performance (and is also why JCSMP doesn't support it). Since we're using Persistent messaging you'd essentially be waiting for the message to be sent, reach the broker and receive and ACK before being able to send the next message on that same producer flow. And that round-trip network time can be brutal!

It looks like the Spring team is adding a new preferred way of handling publisher confirms (using a correlation data header) with the Rabbit Binder in the 3.1 release. They currently have a release candidate out for it so we'll probably want to explore that and maybe talk with the Spring team and brainstorm what options we should add here.

FYI @Nephery.

@GreenRover
Copy link
Contributor

@Mrc0113 Hi Marc, nice to hear. I am highly interested in news according to this issue. Please keep us up to date! At least to put through the exceptions from jcsmp woulp help mutch.

@GreenRover
Copy link
Contributor

GreenRover commented Jan 21, 2021

@Mrc0113 I will start to investigate into this now. Will use https://github.com/SolaceDev/solace-spring-cloud/tree/client-ack as base branch.
Any idea when client-ack make its way into 2.0.0 ?

@Nephery
Copy link
Collaborator Author

Nephery commented Jan 21, 2021

@GreenRover The client-ack branch is already merged with PR #46. Please use stage-2.0.0 for your base branch.

@GreenRover
Copy link
Contributor

As we will not find any good solution in: spring-cloud/spring-cloud-stream#2091
we may end up with:

Possible solution Z:

Add a binder option that enables:

Use a future in:
com.solace.spring.cloud.stream.binder.outbound.JCSMPOutboundMessageHandler#handleMessage

to block and wait for the async JCSMPStreamingPublishEventHandler.responseReceived

Or any better ideas?

@GreenRover
Copy link
Contributor

@Mrc0113 I implemented the "publisher confirms" from rabbit binder to solace
#61
and a options to be totally blocking.

@PhilippeKhalife
Copy link
Contributor

@Nephery to check the relevance of this issue given the most recent features.

@Nephery
Copy link
Collaborator Author

Nephery commented Oct 21, 2021

@PhilippeKhalife This issue is still relevant, though the conversation tangented to implementing publisher confirmations. I've updated the issue description to clarify what this issue is for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants