DATAGO-67282: NACK Support for Consumer bindings #270
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
mayur-solace
commented
Mar 11, 2024
mayur-solace
commented
- DATAGO-67282: Added NACK Support feature, removed ITs related to rebind
- DATAGO-67282: Removed stale tests related to rebind
- DATAGO-67282: Added IT, Removed Commented Code
- DATAGO-67282: Added javadocs
- DATAGO-67282: Updated the version in pom. Updated the spring-cloud-dependencies version and sol-jcsmp version
- DATAGO-67282: Remove unused consumer binding properties flowPreRebindWaitTimeout, flowRebindBackOffInitialInterval, flowRebindBackOffMaxInterval, flowRebindBackOffMultiplier
* DATAGO-67282: Added NACK Support feature, removed ITs related to rebind * DATAGO-67282: Removed stale tests related to rebind * DATAGO-67282: Added IT, Removed Commented Code * DATAGO-67282: Added javadocs * DATAGO-67282: Updated the version in pom. Updated the spring-cloud-dependencies version and sol-jcsmp version * DATAGO-67282: Remove unused consumer binding properties flowPreRebindWaitTimeout, flowRebindBackOffInitialInterval, flowRebindBackOffMaxInterval, flowRebindBackOffMultiplier
Nephery
requested changes
Mar 11, 2024
Nephery
requested changes
Mar 11, 2024
|
|
||
| Refer to <<Failed Consumer Message Error Handling>> for more info. | ||
|
|
||
| | REQUEUE | ||
| | If the consumer is in a defined consumer group, rebind the consumer flow. Otherwise, a `SolaceAcknowledgmentException` will be thrown. | ||
| | For both, the consumer in a defined consumer group or in an anonymous group, signal the Solace broker to requeue/redeliver the message. The message will be redelivered until it is `ACCEPTed` or the message’s max redelivery count is exceeded. |
There was a problem hiding this comment.
The message will be redelivered until it is
ACCEPTedor the message’s max redelivery count is exceeded.
I'm not sure if we should be this specific about what the broker does when we signal a REQUEUE settlement. Since how the broker handles settlements is beyond the binder's scope of responsibility.
solace-spring-cloud-starters/solace-spring-cloud-stream-starter/README.adoc
Outdated
Show resolved
Hide resolved
Comment on lines
+981
to
+982
| 1. Say the current message is marked for 'REQUEUE'. Any subsequent messages that are currently spooled on the client side, despite having been acknowledged `ACCEPTed` by binder, the Solace broker will discard their ACK. | ||
| 2. The Solace Broker will redeliver all messages starting with the one tagged as 'REQUEUE', if the message's max redelivery count is not exceeded. |
There was a problem hiding this comment.
Same question as the other PS+ broker question: Should we be this specific about what the broker does when handling settlements?
IMO I think it might be better to have a short description about this and link to the official docs that describes these details (assuming it exists).
Nephery
requested changes
Mar 11, 2024
...e/src/main/java/com/solace/spring/cloud/stream/binder/inbound/acknowledge/SolaceAckUtil.java
Outdated
Show resolved
Hide resolved
...der-core/src/main/java/com/solace/spring/cloud/stream/binder/util/FlowReceiverContainer.java
Outdated
Show resolved
Hide resolved
...der-core/src/main/java/com/solace/spring/cloud/stream/binder/util/FlowReceiverContainer.java
Outdated
Show resolved
Hide resolved
...der-core/src/main/java/com/solace/spring/cloud/stream/binder/util/FlowReceiverContainer.java
Outdated
Show resolved
Hide resolved
Nephery
requested changes
Mar 12, 2024
...n/java/com/solace/spring/cloud/stream/binder/util/UnboundFlowReceiverContainerException.java
Show resolved
Hide resolved
...d-stream-binder/src/test/java/com/solace/spring/cloud/stream/binder/SolaceBinderBasicIT.java
Outdated
Show resolved
Hide resolved
...ream-binder/src/test/java/com/solace/spring/cloud/stream/binder/SolaceBinderClientAckIT.java
Show resolved
Hide resolved
...ream-binder/src/test/java/com/solace/spring/cloud/stream/binder/SolaceBinderClientAckIT.java
Outdated
Show resolved
Hide resolved
...test/java/com/solace/spring/cloud/stream/binder/SolaceBinderCustomErrorMessageHandlerIT.java
Outdated
Show resolved
Hide resolved
...est/java/com/solace/spring/cloud/stream/binder/util/ErrorQueueRepublishCorrelationKeyIT.java
Show resolved
Hide resolved
…er. Adjusted related tests. Refactored SolaceAckUtil
Nephery
requested changes
Mar 14, 2024
...e/src/main/java/com/solace/spring/cloud/stream/binder/inbound/InboundXMLMessageListener.java
Show resolved
Hide resolved
...core/src/main/java/com/solace/spring/cloud/stream/binder/util/SolaceErrorMessageHandler.java
Outdated
Show resolved
Hide resolved
...re/src/main/java/com/solace/spring/cloud/stream/binder/util/JCSMPSessionProducerManager.java
Outdated
Show resolved
Hide resolved
...re/src/main/java/com/solace/spring/cloud/stream/binder/util/JCSMPSessionProducerManager.java
Outdated
Show resolved
Hide resolved
Nephery
requested changes
Mar 14, 2024
...solace/spring/cloud/stream/binder/inbound/acknowledge/JCSMPBatchAcknowledgementCallback.java
Outdated
Show resolved
Hide resolved
Nephery
approved these changes
Mar 15, 2024
There was a problem hiding this comment.
This looks good now. Thanks @mayur-solace !
That +1,191 −4,348 line changes is pretty satisfying to see 😄
Nephery
added a commit
that referenced
this pull request
Apr 3, 2024
* DATAGO-67282: NACK Support for Consumer bindings (#270) * DATAGO-67282: Added NACK Support feature, removed ITs related to rebind * DATAGO-67282: Removed stale tests related to rebind * DATAGO-67282: Added IT, Removed Commented Code * DATAGO-67282: Added javadocs * DATAGO-67282: Updated the version in pom. Updated the spring-cloud-dependencies version and sol-jcsmp version * DATAGO-67282: Remove unused consumer binding properties flowPreRebindWaitTimeout, flowRebindBackOffInitialInterval, flowRebindBackOffMaxInterval, flowRebindBackOffMultiplier * Update BOM README for 4.0.0 Release (#274) * DATAGO-72125: Logger change to not print stacktrace. (#275) * DATAGO-72125: Logger changes (#276) * DATAGO-71389: Solace Binder 5.0 version upgrade/migration guide (#277) * DATAGO-67282: Resolve merge conflicts * DATAGO-67282 Resolve Merge Conflicts with master (#281) * Release 3.2.0 (#279) (#280) * DATAGO-67277: Feature to exclude header on consumer binding Co-authored-by: mayur-solace <81177023+mayur-solace@users.noreply.github.com> * DATAGO-67282: Resolve merge conflicts --------- Co-authored-by: Jeffrey D <11084623+Nephery@users.noreply.github.com> * Revert "DATAGO-67282 Resolve Merge Conflicts with master (#281)" (#282) This reverts commit 114ab2e. * DATAGO-67282: Update the Spring Boot and Spring Cloud Version (#287) --------- Co-authored-by: mayur-solace <81177023+mayur-solace@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.