1. "Added NONE delivery semantics and delete autoAck config." - Added --> add
2. I suggest calling it `MANUAL` `ProcessingGuarantee` instead of NONE. As you carefully explained, ProcessingGuarantee comes does to the fact that the function executor calls acknowledge, in specific timing: - `AT_MOST_ONCE` - When the message is read by the client, it is immediately acknowledged and only then the function is executed, thus guaranteeing it will not run more than once - `AT_LEAST_ONCE` - Message is acknowledged *after* the function finished execution, thus it will be run at least once. - `MANUAL` - Signals to the user that it is up to them to acknowledge the message, inside the function. I think if you couple that change with adding the explanation I wrote above to the documentation it will become crystal clear (hopefully) what is a Processing Guarantee exactly and what each value signifies. 3. Removing autoAck from Function Config breaks backward compatibility, thus shouldn't this be strongly reflected in the PIP - how does Pulsar release handle breaking change? 4. Regarding Implementation (1), isn't the class itself PulsarSinkAtLeastOnceProcessor encodes what to do? I'm not sure I understand how you use that enum value *inside* the class/ On Fri, May 27, 2022 at 1:08 AM Neng Lu <freen...@gmail.com> wrote: > Some suggestions: > > 1. Instead of deleting the `autoAck`, keep it but not use it in the code. > And documented clearly it's deprecated for the following 2~3 release. And > then delete it. > 2. For `PulsarSinkAtLeastOnceProcessor` and > `PulsarSinkEffectivelyOnceProcessor`, if `NONE` is configured, it defaults > to ATLEAST_ONCE. > 3. Need to let users know the behavior when they call `record.ack()` inside > the function implementation. > > On Thu, May 12, 2022 at 1:52 AM Baozi <wudixiaolong...@icloud.com.invalid> > wrote: > > > Hi Pulsar community, > > > > I open a https://github.com/apache/pulsar/issues/15560 for Function add > > NONE delivery semantics > > > > Let me know what you think. > > > > > > Thanks, > > Baodi Shi > > > > > > ## Motivation > > > > Currently Function supports three delivery semantics, and also provides > > autoAck to control whether to automatically ack. > > Because autoAck affects the delivery semantics of Function, it can be > > confusing for users to understand the relationship between these two > > parameters. > > > > For example, when the user configures `Guarantees == ATMOST_ONCE` and > > `autoAck == false`, then the framework will not help the user to ack > > messages, and the processing semantics may become `ATLEAST_ONCE`. > > > > The delivery semantics provided by Function should be clear. When the > user > > sets the guarantees, the framework should ensure point-to-point semantic > > processing and cannot be affected by other parameters. > > > > ## Goal > > > > Added `NONE` delivery semantics and delete `autoAck` config. > > > > The original intention of `autoAck` semantics is that users want to > > control the timing of ack by themselves. When autoAck == false, the > > processing semantics provided by the framework should be invalid. Then we > > can add `NONE` processing semantics to replace the autoAck == false > > scenario. > > > > When the user configuration `ProcessingGuarantees == NONE`, the framework > > does not help the user to do any ack operations, and the ack is left to > the > > user to handle. In other cases, the framework guarantees processing > > semantics. > > > > ## API Changes > > 1. Add `NONE` type to ProcessingGuarantees > > ``` java > > public enum ProcessingGuarantees { > > ATLEAST_ONCE, > > ATMOST_ONCE, > > EFFECTIVELY_ONCE, > > NONE > > } > > ``` > > > > 2. Delete autoAck config in FunctionConfig > > ``` java > > public class FunctionConfig { > > - private Boolean autoAck; > > } > > ``` > > > > ## Implementation > > > > 1. In `PulsarSinkAtLeastOnceProcessor` and > > `PulsarSinkEffectivelyOnceProcessor`, when `ProcessingGuarantees != NONE` > > can be ack. > > > > < > > > https://github.com/apache/pulsar/blob/c49a977de4b0b525ec80e5070bc90eddcc7cddad/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/sink/PulsarSink.java#L274-L276 > > > > > > > 2. When the delivery semantic is `ATMOST_ONCE`, the message will be acked > > immediately after receiving the message, no longer affected by the > autoAck > > configuration. > > > > > > > https://github.com/apache/pulsar/blob/c49a977de4b0b525ec80e5070bc90eddcc7cddad/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L271-L276 > > > > 3. When user call `record.ack()` in function, just `ProcessingGuarantees > > == NONE` can be work. > > > > ## Plan test > > The main test and assert is that when ProcessingGuarantees == NONE, the > > function framework will not do any ack operations for the user. > > > > ## Compatibility > > 1. This change will invalidate the user's setting of autoAck, which > should > > be explained in the documentation and provide parameter verification to > > remind the user. > > 2. Runtimes of other languages need to maintain consistent processing > > logic (python, go). > > > > > > > > -- > Best Regards, > Neng >
