Re: Question about excluding serialized classes

[Aaron Lindsey]

Hi Jake,

I think this thread has moved a bit off-topic, but I appreciate you
pointing out that scenario. We'll have to consider that when re-working
this PR.

- Aaron


On Tue, Sep 17, 2019 at 9:42 AM Jacob Barrett  wrote:

>
> > On Sep 17, 2019, at 9:36 AM, Aaron Lindsey  wrote:
> >
> >>
> >> 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?
> >>
> >
> > For this meter we're only interested in timing registered functions.
>
> Can you explain why ya’ll aren’t interested in the more common use case?
>
> Also, are you aware this will likely result in a mismatch in meters for
> the same function between the client and the server. If the client has the
> function registered and the function is executed by name then the internals
> fetch the registered function object and effectively executes as
> Execution.execute(Function). This means that the client side will update
> meters for this function but when deserialized on the server to execute it
> would not update meters on the server. Only the case where the client
> doesn't have the Function registered and executes by name but the server
> does have it registered and looks up the instance would you end up with
> metrics updated on both sides.
>
> -Jake
>
>

Re: Question about excluding serialized classes

[Jacob Barrett]

> On Sep 17, 2019, at 9:36 AM, Aaron Lindsey  wrote:
> 
>> 
>> 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?
>> 
> 
> For this meter we're only interested in timing registered functions.

Can you explain why ya’ll aren’t interested in the more common use case?

Also, are you aware this will likely result in a mismatch in meters for the 
same function between the client and the server. If the client has the function 
registered and the function is executed by name then the internals fetch the 
registered function object and effectively executes as 
Execution.execute(Function). This means that the client side will update meters 
for this function but when deserialized on the server to execute it would not 
update meters on the server. Only the case where the client doesn't have the 
Function registered and executes by name but the server does have it registered 
and looks up the instance would you end up with metrics updated on both sides.

-Jake

Re: Question about excluding serialized classes

[Aaron Lindsey]

>
> 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?
>

For this meter we're only interested in timing registered functions.
- Aaron


On Tue, Sep 17, 2019 at 9:18 AM Dale Emery  wrote:

>
> > On Sep 16, 2019, at 7:54 PM, Jacob Barrett  wrote:
> >
> > The current implementation has statistics without decoration.
>
> For our meters, we want to make a distinction that the current stats
> implementation does not: We want to measure only non-internal functions. It
> isn’t clear (yet) how we can inform the function stats constructor so that
> it knows whether to create the meters.
>
> Cheers,
> Dale
>
> —
> Dale Emery
> dem...@pivotal.io
>
>

Re: Question about excluding serialized classes

[Dale Emery]

> On Sep 16, 2019, at 7:54 PM, Jacob Barrett  wrote:
> 
> The current implementation has statistics without decoration.

For our meters, we want to make a distinction that the current stats 
implementation does not: We want to measure only non-internal functions. It 
isn’t clear (yet) how we can inform the function stats constructor so that it 
knows whether to create the meters.

Cheers,
Dale

—
Dale Emery
dem...@pivotal.io

Re: Question about excluding serialized classes

[Jacob Barrett]

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  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  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  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  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 
>> 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  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 
>> wrote:
>> 
>>> As part of a PR to add Micrometer timers for function executions
>>> , 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
>> 

Re: Question about excluding serialized classes

[Aaron Lindsey]

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  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  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  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 
> 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  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 
>  wrote:
> 
> > As part of a PR to add Micrometer timers for function executions
> > , 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
> >
> 
> >>
> >
>
>

Re: Question about excluding serialized classes

[Jacob Barrett]

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  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  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  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  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 
 wrote:
 
> As part of a PR to add Micrometer timers for function executions
> , 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
> 
 
>> 
> 

Re: Question about excluding serialized classes

[Dale Emery]

As far as I can tell, the things that execute functions use the public API to 
find the function to execute. So if we unwrap the functions in the public API, 
only the un-instrumented functions will be executed.

—
Dale Emery
dem...@pivotal.io



> On Sep 11, 2019, at 1:38 PM, Dan Smith  wrote:
> 
> I think you could still use your decorator approach if you also unwrap the
> Functions when returning them from the public APIs like getFunction etc. In
> that case your TimingFunction could probably safely by added to
> excludedClasses.txt.
> 
> You will miss collecting metrics about functions that aren't registered and
> are invoked using Execution.execute(Function) but maybe that's intentional?
> 
> -Dan
> 
> On Wed, Sep 11, 2019 at 1:24 PM Mark Hanson  wrote:
> 
>> They would be the specific functions, but this doesn’t get us around they
>> other problem. I think this approach is not going to work and we are about
>> to start looking into other ways.
>> 
>> Thanks,
>> Mark
>> 
>>> On Sep 11, 2019, at 12:14 PM, Anthony Baker  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 
>> 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  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 
> wrote:
> 
>> As part of a PR to add Micrometer timers for function executions
>> , 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
>> 
> 
>>> 
>> 
>> 

Re: Question about excluding serialized classes

[Dale Emery]

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  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  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  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 
>>> wrote:
>>> 
 As part of a PR to add Micrometer timers for function executions
 , 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
 
>>> 
> 

Re: Question about excluding serialized classes

[Dan Smith]

I think you could still use your decorator approach if you also unwrap the
Functions when returning them from the public APIs like getFunction etc. In
that case your TimingFunction could probably safely by added to
excludedClasses.txt.

You will miss collecting metrics about functions that aren't registered and
are invoked using Execution.execute(Function) but maybe that's intentional?

-Dan

On Wed, Sep 11, 2019 at 1:24 PM Mark Hanson  wrote:

> They would be the specific functions, but this doesn’t get us around they
> other problem. I think this approach is not going to work and we are about
> to start looking into other ways.
>
> Thanks,
> Mark
>
> > On Sep 11, 2019, at 12:14 PM, Anthony Baker  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 
> 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  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 
> >>> wrote:
> >>>
>  As part of a PR to add Micrometer timers for function executions
>  , 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
> 
> >>>
> >
>
>

Re: Question about excluding serialized classes

[Mark Hanson]

They would be the specific functions, but this doesn’t get us around they other 
problem. I think this approach is not going to work and we are about to start 
looking into other ways.

Thanks,
Mark

> On Sep 11, 2019, at 12:14 PM, Anthony Baker  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  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  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 
>>> wrote:
>>> 
 As part of a PR to add Micrometer timers for function executions
 , 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
 
>>> 
> 

Re: Question about excluding serialized classes

[Anthony Baker]

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  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  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 
>> wrote:
>> 
>>> As part of a PR to add Micrometer timers for function executions
>>> , 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
>>> 
>> 

Re: Question about excluding serialized classes

[Dan Smith]

Yeah, I would expect that FunctionService.getFunction() would return the
same function object I registered with FunctionService.registerFunction.

-Dan

On Wed, Sep 11, 2019 at 12:01 PM Aaron Lindsey  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  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 
> > wrote:
> >
> > > As part of a PR to add Micrometer timers for function executions
> > > , 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
> > >
> >
>

Re: Question about excluding serialized classes

[Aaron Lindsey]

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  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 
> wrote:
>
> > As part of a PR to add Micrometer timers for function executions
> > , 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
> >
>

Re: Question about excluding serialized classes

[Dan Smith]

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  wrote:

> As part of a PR to add Micrometer timers for function executions
> , 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
>