>> This I don't understand fully. When you introduce MANUAL > ProcessingGuarantee configuration, isn't configuration universal to all > languages? When someone uses pulsar-admin to create a function, regardless > of the language, they can specify Processing Guarantee. If we're adding > this ability and documenting it, isn't supporting all clients mandatory? I > definitely think it can be separate pull requests, but it seems important > IMO to deliver consistent developer experience to the person developing > functions.
Yes, to support all clients. I didn't express it clearly before. Since our changes are backward compatible, before the change is released, we can open multiple PR to iteratively implement the runtime of each language. When all languages are supported, publish documentation to inform users. Thanks, Baodi Shi > 2022年5月30日 22:0613，Asaf Mesika <asaf.mes...@gmail.com> 写道： > > On Mon, May 30, 2022 at 4:24 PM Baozi <wudixiaolong...@icloud.com.invalid> > wrote: > >> Hi, Rui. >> >> Thanks review. >> >>> 1. API changes should also contain the changes of `Function.proto`, >> including new `ProcessingGuarantees` option and `autoAck`. >> >> >> I added to pip. >> >>> 2. Please be sure the other language runtimes (like Python, Golang) do >> support similar `record.ack()` function from the context, if no, it might >> have some API changes for different runtime we well. >> >> >> I added to Compatibility. The goal of this PIP is to keep other language >> runtimes consistent with java, but it needs to be iterated slowly. We will >> support java runtimes first. >> >> This I don't understand fully. When you introduce MANUAL > ProcessingGuarantee configuration, isn't configuration universal to all > languages? When someone uses pulsar-admin to create a function, regardless > of the language, they can specify Processing Guarantee. If we're adding > this ability and documenting it, isn't supporting all clients mandatory? I > definitely think it can be separate pull requests, but it seems important > IMO to deliver consistent developer experience to the person developing > functions. > > > >> Thanks, >> Baodi Shi >> >>> 2022年5月30日 12:5128，Rui Fu <r...@apache.org> 写道： >>> >>> Hi Baodi, >>> >>> Nice work. Put some suggestions below, ptal. >>> >>> 1. API changes should also contain the changes of `Function.proto`, >> including new `ProcessingGuarantees` option and `autoAck`. >>> 2. Please be sure the other language runtimes (like Python, Golang) do >> support similar `record.ack()` function from the context, if no, it might >> have some API changes for different runtime we well. >>> >>> >>> Best, >>> >>> Rui Fu >>> 在 2022年5月29日 +0800 22:18，Asaf Mesika <asaf.mes...@gmail.com>，写道： >>>> 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 >>>>> >> >>
