Not all functions are registered. I can invoke a function with
Execution.execute(Function) from the client, the Function is serialized and
executed on the server, the class need only exist on the server for
deserialization. Must a function be registered now to get metrics?
> On Sep 17, 2019, at 8:44 AM, Aaron Lindsey <alind...@pivotal.io> wrote:
>
> Jake — We're not constructing a new object each time a function is invoked
> because the "decorated" functions are only created when a function is
> registered, but we'll keep your concern in mind.
>
> Thanks for all of the feedback from everyone. I think we have the
> information we need to move forward. In summary, we'll have to change our
> implementation such that the methods in FunctionService still return the
> same objects they did before our changes.
>
> - Aaron
>
>
> On Mon, Sep 16, 2019 at 7:54 PM Jacob Barrett <jbarr...@pivotal.io> wrote:
>
>> Sorry I am entering this late in the game but why do we need to decorate
>> the function at all for metrics. The current implementation has statistics
>> without decoration. All the concerns raised in this thread concern me as
>> well as the cost of constructing yet another object every time a function
>> is invoked by serialized function, Execution.execute(Function). Each
>> allocation puts more pressure on the GC, decreases our throughput and
>> increases our latency.
>>
>> -Jake
>>
>>
>>> On Sep 11, 2019, at 2:29 PM, Dale Emery <dem...@pivotal.io> wrote:
>>>
>>> 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
>>>>>>>
>>>>>>
>>>>
>>>
>>
>>
