- Regarding the interface design: One thing we need to confirm is whether we **don't want old runtime to execute new function without proper initialization** or not.
If this rule must be followed, then one possible way is we define a separate `HookFunction` interface which includes `setup`, `process`, `tearDown` methods, and add support in the new runtime to handle this interface. Just as currently the runtime handles `pulsar.Function` and `java.util.Function` separately. The old runtime will not be able to recognize the new interface and thus won't execute functions which need initialization. Documentation and examples can be added to help user learn and understand this new interface. - Regarding the exception: I think user is responsible for properly handle any checked exceptions and thus the interface should not allow throwing any exception. For any unchecked exceptions, the runtime/process will fail fast. - Regarding the contract: Given that checked exceptions are handled by user properly as described above, the `setup` method must be successfully executed or exited. So, there's no need to call the `tearDown` method. On 2021/07/06 21:02:17 Enrico Olivelli wrote: > Il Mar 6 Lug 2021, 21:30 Neng Lu <nl...@apache.org> ha scritto: > > > IMHO, The old runtime should not be able to run the new functions. > > > This is not possible to enforce this because we cannot change old code. > > > > > > > New functions require resource initialization via hooks. If the actual > > `setup()` method is not called (or only a default no-op one is called), > > then the function is not properly initialized and there'll be problems if > > they are executed. > > > > I believe that using default methods is better, as it is clearer for new > users. > > If you add a new interface then it is difficult for a new user to discover > this feature. > > Additionally we have to define the contract for calling the teardown > function. > Will it be called in case of failure of the setup function? > > Are those functions allowed to throw exceptions. I believe it is good to > let them throw Exception in order to not force the user to catch everything > and re throw as a RuntimeException > > Enrico > > > > > > On 2021/07/06 19:03:09 Sijie Guo wrote: > > > So there are two compatibility issues we need to consider here. > > > > > > 1) Old runtime to run new functions. > > > 2) New runtime to run old functions. > > > > > > Making the methods with default no-op implementation will resolve 1). > > > Is that correct? > > > > > > We can use reflection to check if the methods exist or not to solve 2), > > no? > > > > > > - Sijie > > > > > > On Tue, Jul 6, 2021 at 11:34 AM Neng Lu <nl...@apache.org> wrote: > > > > > > > > I think the reason is for keeping the original `Function` unchanged, > > so that existing implemented functions are not affected. > > > > > > > > > > > > On 2021/07/06 03:34:49 Sijie Guo wrote: > > > > > Thank you for starting the discussion! > > > > > > > > > > I have added this proposal to PIP-86: > > > > > > > https://github.com/apache/pulsar/wiki/PIP-86:-Pulsar-Functions:-Preload-and-release-external-resources > > > > > > > > > > I have one question: why do you introduce HookFunction? Why not just > > add > > > > > two default methods to the existing Functions API? > > > > > > > > > > - Sijie > > > > > > > > > > On Mon, Jul 5, 2021 at 6:49 PM 陈磊 <chen...@cmss.chinamobile.com> > > wrote: > > > > > > > > > > > Motivation > > > > > > > > > > > > It is very useful in many scenarios to provide safe and convenient > > > > > > capabilities for function's external resource initialization and > > release. > > > > > > In addition to the normal data processing path, it enables > > functions to use > > > > > > HookFunction to manage external resources > > > > > > > > > > > > At present, in order to process data, only the logic of resource > > > > > > initialization -> processing -> release and shutdown can be > > written in the > > > > > > process() of Function. This method is complicated, insecure, and > > > > > > unnecessary. > > > > > > > > > > > > Instead, we should have a new standard way for users to use > > Function > > > > > > easily and safely. Summarized as follows: > > > > > > > > > > > > Before Function starts, some resources only need to be initialized > > once, > > > > > > and there is no need to make various judgments in the process() > > method of > > > > > > the Function interface > > > > > > > > > > > > After closing the Function, in the process of using process(), you > > need to > > > > > > manually close the referenced external resources, which need to be > > released > > > > > > separately in the close() of javaInstance > > > > > > API and Implementation Changes > > > > > > > > > > > > The organization of the function implementation hierarchy has been > > added, > > > > > > it currently looks like the following figure: > > > > > > > > > > > > Use Cases: > > > > > > > > > > > > Before transformation > > > > > > public class DemoFunction implements Function<String, String>{ > > > > > > RedisClient client; > > > > > > @Override > > > > > > public String process(String str, Context context) { > > > > > > 1.client=init(); > > > > > > 2.Object object = client.get(key); > > > > > > //Data enrichment > > > > > > 3.client.close(); > > > > > > return null; > > > > > > } > > > > > > } > > > > > > > > > > > > After the transformation > > > > > > public class DemoFunction implements HookFunction<String, String>{ > > > > > > RedisClient client; > > > > > > @Override > > > > > > public void setup(Context context) { > > > > > > Map<String, Object> connectInfo = > > context.getUserConfigMap(); > > > > > > client=init(); > > > > > > } > > > > > > > > > > > > @Override > > > > > > public String process(String str, Context context) { > > > > > > Object object = client.get(key); > > > > > > //Data enrichment > > > > > > return null; > > > > > > } > > > > > > > > > > > > @Override > > > > > > public void cleanup() { > > > > > > client.close(); > > > > > > } > > > > > > } > > > > > > > > > > > > It is quite simple and clear to use in function processing code. > > > > > > <http://conf.cmss.com/pages/viewpage.action?pageId=140726144> > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
