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

Fix #63 - Clarify the NONE ack policy usage #66

Merged
merged 1 commit into from Sep 3, 2019

Conversation

cescoffier
Copy link
Contributor

No description provided.

@cescoffier cescoffier added this to the 1.0.1 milestone Sep 2, 2019
@@ -699,6 +698,10 @@ public Message<Integer> process(KafkaMessage<String, Integer> input) {
}
----

The `NONE` acknowledgment strategy indicates that the message is not acknowledged by the method (neither implicitly nor explicitly).
However, note that all incoming messages must be acknowledged, so using `NONE` indicates that the message is acknowledged in a different location or using a different mechanism.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could benefit from something more to differentiate it from "MANUAL:managed by the user code". Do you mean that myMessage.ack() still needs to be called but just not in this method or is it referring to Kafka layer acknowledgement must still be done for example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is difficult to explain - without introducing the concept that we may be sending 1 message to more than 1 receiver (which is a concept that is not in the spec currently).

Copy link
Member

@hutchig hutchig Sep 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the difference is in effect then it might help to explain what the effect ( of having NONE versus MANUAL) is expected to be. For example.
"Specifying 'NONE' allows for this method to complete processing and return without handling acknowledgement and for this to be considered valid behaviour of the method.
However, if messages were never acknowledged this would result in
a build up of unacknowledged messages in the system as no
automatic acknowledgement will be done when 'NONE' is specified.
When 'NONE' is specified, each message object's ack() method should still be invoked
once as part of the overall processing but this is considered valid behaviour
either before, during or after this method's execution."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Specifying 'NONE' allows for this method to complete processing and return without handling acknowledgment and for this to be considered valid behavior of the method.

Yes.

When 'NONE' is specified, each message object's ack() method should still be invoked
once as part of the overall processing but this is considered valid behaviour
either before, during or after this method's execution."

Exactly

I'm going to update the PR with this content.

Copy link
Member

@hutchig hutchig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Specifying 'NONE' allows for this method to complete processing and return without handling acknowledgement and for this to be considered valid behaviour of the method.
However, if messages were never acknowledged the NONE policy would result in
a build up of unacknowledged messages in the system as no
automatic acknowledgement will be done when 'NONE' is specified.
When 'NONE' is specified, each message object's ack() method should still be invoked
once as part of the overall processing but this is considered valid behaviour
either before, during or after this method's execution."

@@ -699,6 +698,10 @@ public Message<Integer> process(KafkaMessage<String, Integer> input) {
}
----

The `NONE` acknowledgment strategy indicates that the message is not acknowledged by the method (neither implicitly nor explicitly).
However, note that all incoming messages must be acknowledged, so using `NONE` indicates that the message is acknowledged in a different location or using a different mechanism.
Copy link
Member

@hutchig hutchig Sep 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the difference is in effect then it might help to explain what the effect ( of having NONE versus MANUAL) is expected to be. For example.
"Specifying 'NONE' allows for this method to complete processing and return without handling acknowledgement and for this to be considered valid behaviour of the method.
However, if messages were never acknowledged this would result in
a build up of unacknowledged messages in the system as no
automatic acknowledgement will be done when 'NONE' is specified.
When 'NONE' is specified, each message object's ack() method should still be invoked
once as part of the overall processing but this is considered valid behaviour
either before, during or after this method's execution."

Signed-off-by: Clement Escoffier <clement.escoffier@gmail.com>
@cescoffier
Copy link
Contributor Author

@hutchig I've updated the PR with your text.

Copy link
Member

@hutchig hutchig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cescoffier cescoffier merged commit fc1bd3a into master Sep 3, 2019
@cescoffier cescoffier deleted the features/clarify-ack-none branch September 3, 2019 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants