The stats use the ID of the function, and each TimingFunction reports the same 
ID as the function it wraps. So I think the stats would look like they always 
did.

Dale

—
Dale Emery
dem...@pivotal.io



> On Sep 11, 2019, at 12:14 PM, Anthony Baker <aba...@pivotal.io> wrote:
> 
> 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
>>>> 
>>> 
> 

Reply via email to