I think the Decorator approach you outlined could have other impacts as well.
Would I still be able to see specific function executions in statistics or
would they all become “TImingFunction”?
Anthony
> On Sep 11, 2019, at 12:00 PM, Aaron Lindsey <alind...@pivotal.io> wrote:
>
> Thanks for your response, Dan.
>
> The second scenario you mentioned (i.e. users calling
> FunctionService.getFunction(String)) worries me because our PR changes the
> FunctionService so they would now get back an instance of the decorator
> function instead of the specific instance they registered by calling
> FunctionService.registerFunction(Function). Therefore, any explicit casts
> to a Function implementation like (MyFunction)
> FunctionService.getFunction("MyFunction") would fail. Does that mean this
> be a breaking change? The FunctionService class does not specify that
> getFunction must return the same type function as the one passed to
> registerFunction, but I could see how users might be relying on that
> behavior since there is no other way to get a specific function type out of
> the FunctionService without doing a cast.
>
> - Aaron
>
>
> On Wed, Sep 11, 2019 at 10:52 AM Dan Smith <dsm...@pivotal.io> wrote:
>
>> Functions are serialized when you call Execution.execute(Function) instead
>> of Execution.execute(String). Invoking execute on a function object
>> serializes the function and executes it on the remote side. Functions
>> executed this way don't have be registered.
>>
>> Users can also get registered function objects directly from the function
>> service using FunctionService.getFunction(String) and do whatever they want
>> with them, which I guess could include serializing them.
>>
>> Hope that helps!
>> -Dan
>>
>> On Wed, Sep 11, 2019 at 10:27 AM Aaron Lindsey <alind...@pivotal.io>
>> wrote:
>>
>>> As part of a PR to add Micrometer timers for function executions
>>> <https://github.com/apache/geode/pull/4038>, we implemented a decorator
>>> Function that wraps all registered non-internal functions and adds
>>> instrumentation. This PR is
>>> failing AnalyzeSerializablesJUnitTest.testSerializables because the
>>> decorator class is a new Serializable.
>>>
>>> I'm not sure if it would be OK to add this class to excludedClasses.txt
>>> because I don't know whether this function will ever be serialized. If it
>>> will be serialized, then I'm concerned that this might break backwards
>>> compatibility because we're changing the serialized form of registered
>>> functions. If this is the case, then we could implement custom logic for
>>> serializing the decorator class which would replace its serialized form
>>> with the serialized form of the inner class. Again, I'm not sure if that
>>> would be necessary because I don't know the conditions under which a
>>> function would be serialized.
>>>
>>> Could someone help me understand when functions would be persisted or
>> sent
>>> over the wire so I can determine if this change would break
>> compatibility?
>>>
>>> Thanks,
>>> Aaron
>>>
>>
