You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Udo Kohlmeyer <ud...@apache.org> on 2018/03/07 19:18:06 UTC
FunctionService Proposal
Hi there Apache Dev's,
Please look at the proposal to improve the FunctionService and remove
the static invocation of it from within the Cache.
https://cwiki.apache.org/confluence/display/GEODE/Function+Service+Refactor+-+Removal+of+static-ness+and+splitting+of+client+and+server-side+FunctionService
--Udo
Re: FunctionService Proposal
Posted by Jacob Barrett <jb...@pivotal.io>.
I agree with Dan here. The C++ client took a stab at making
ExecutionService less static but didn't go this far. Rather than pull Cache
out of the either it is passed into each of the static factory methods,
wither as a Region, Pool or Cache itself. I like propose to add
Cache::getFunctionService to cache.
I find UML diagrams to be a bit heavy handed for describing APIs. Can we
just simplify this into so sample code that users will actually interact
with.
So if we were to make this change on the C++ side we would have that (C++
Cache is client cache, confusing yes).
class Cache ... {
...
FunctionService& getFunctionService();
}
class FunctionService ... {
...
Execution& onRegion(const std::string& regionName);
Execution& onRegion(std::shared_ptr<Region> region);
Execution& onServers(std::shared_ptr<Pool> pool);
Execution& onServers(); // was onServer(std::shared_ptr<RegionService>)
Execution& onServer(std::shared_ptr<Pool> pool);
Execution& onServer(); // was onServer(std::shared_ptr<RegionService>)
}
class Region ... {
...
Execution& createExecution(); // nice shortcut
}
void doExecution(Cache& cache) {
auto results = cache.getExecutionService()
.onRegion("myRegion")
.withArgs(MyFunctionArgs{"arg1", 2})
.execute("MyFunction");
...
}
void doExecution(Region& cache) {
auto results = region.createExecution()
.withArgs(MyFunctionArgs{"arg1", 2})
.execute("MyFunction");
...
}
On a related note, I would suggest we use the unqualified names for the
client since that is where real "users" of our API will be spending most of
their time. For them calling everything ClientX and ClientY is a bit
redundant.
On Wed, Mar 7, 2018 at 1:45 PM Dan Smith <ds...@pivotal.io> wrote:
> Hi Udo,
>
> +1 for making the function service not static and spitting it into client
> and server FunctionService objects!
>
> We do have Cache and ClientCache right now. So I would recommend this API
> rather than putting two methods on Cache. Cache is already the the server
> side API.
>
> Cache {
> ServerFunctionService getFunctionService()
> }
>
> ClientCache {
> ClientFunctionService getFunctionService()
> }
>
> If at some point we split the client side API into a separate jar the API
> shouldn't need to change. If you don't like ClientFunctionService, maybe
> o.a.g.cache.client.execute.FunctionService? We would never want two
> different versions of org.apache.geode.function.FunctionService. People
> wouldn't be able to test a client and server in the same JVM.
>
> Also, you might want to check out this (somewhat stalled) proposal -
> https://cwiki.apache.org/confluence/display/GEODE/
> Function+Service+Usability+Improvements. We had buy in on this on the dev
> list but have not found cycles to actually do it yet. But maybe now is the
> time?
>
> -Dan
>
> On Wed, Mar 7, 2018 at 11:18 AM, Udo Kohlmeyer <ud...@apache.org> wrote:
>
> > Hi there Apache Dev's,
> >
> > Please look at the proposal to improve the FunctionService and remove the
> > static invocation of it from within the Cache.
> >
> > https://cwiki.apache.org/confluence/display/GEODE/Function+
> > Service+Refactor+-+Removal+of+static-ness+and+splitting+of+
> > client+and+server-side+FunctionService
> >
> > --Udo
> >
> >
>
Re: FunctionService Proposal
Posted by John Blum <jb...@pivotal.io>.
+1 Udo. A very clear and descriptive spec; Nice work!
In short, these would be welcomed changes and would have very little impact
to *Spring Data for Apache Geode*, which nicely abstracts this API even
further using its POJO-based, Annotation configuration model for Function
Implementations as well as Executions.
I too would prefer that there be a single method to obtain an instance of
the FunctionService. However, as we know this is not currently possible
without a clear and proper separation of the client and peer/server cache.
As Dan points out, while there are 2 separate interfaces for client and
peer/server caches, namely o.a.g.cache.client.ClientCache and
o.a.g.cache.Cache, the problem remains, there is only a single
implementation of both, o.a.g.internal.cache.GemFireCacheImpl, and
therefore, it is not possible to have a 2 exactly named methods that only
vary by return type in the same class (this is different than a subclass
having a different, compatible return type, which was only allowed as of
Java 8?).
I propose the following...
interface FunctionService {
onRegion(:Region);
}
interface ClientFunctionService extends FunctionService {
onServer(:Pool);
onServers(:Pool);
onServer(:RegionService);
onServers(:RegionService);
}
interface ServerFunctionService extends FunctionService {
onMember(:String...)
onMembers(:String...)
onMember(:DistributedMember)
onMembers(:DistributedMember[])
}
Then on the GemFireCache interface, you can declare...
interface GemFire extends RegionService {
<T extends FunctionService> T getFunctionService();
...
}
Then it is a simple matter for users to get reference to either, for
example from a client:
ClientFunctionService clientFunctionService =
clientCache.getFunctionService();
Or, from a server (peer):
ServerFunctionService serverFunctionService = cache.getFunctionService();
A (crude) implementation of getFunctionService() in
o.a.g.internal.cache.GemFireCacheImpl, would be...
@SuppressWarnings("unchecked")
public <T extends FunctionService> T getFunctionService() {
return (T) (isClient() ? new DefaultClientFunctionService() : new
DefaultServerFunctionService());
}
Of course, the implementation could be much more sophisticate than this.
The point being, I don't see the separate of the client and server logic
for cache functionality being split anytime soon and this would a good
interim solution that would not require any changes in the future.
I might even make the instantiation of the Client or Server based
FunctionServices accessible from a protected method for extension purpose
(i.e. subclassing), thereby making Apache Geode more extensible, so...
@SuppressWarnings("unchecked")
public <T extends FunctionService> T getFunctionService() {
return (T) (isClient() ? newClientFunctionService() :
newServerFunctionService());
}
protected ClientFunctionService newClientFunctionService() {
return new DefaultClientFunctionService();
}
protected ServerFunctionService newServerFunctionService() {
return new DefaultServerFunctionService();
}
It wouldn't be too difficult to imagine this being accomplished using
Java's SPI as well. Having a customizable/extensible API for Function
execution would be useful where users wanted to plugin different execution
handlers, for streaming or reactive purposes, for instance, and so on. If
the user is not satisfied with the defaults, regardless of concern, they
can plugin their own removing us from the equation. It does not preclude
us from providing OOTB implementations for these concerns, but they would
just be another provider like any other.
Food for thought.
Cheers,
John
On Thu, Mar 8, 2018 at 5:02 PM, Galen O'Sullivan <go...@pivotal.io>
wrote:
> +1 for making the function service not static, and splitting servers from
> clients.
>
> Also +1 for Dan's suggestion.
>
> On Wed, Mar 7, 2018 at 2:51 PM, Patrick Rhomberg <pr...@pivotal.io>
> wrote:
>
> > I did not know that! And then, yes, onRegion is much better.
> >
> > On Wed, Mar 7, 2018 at 2:43 PM, Dan Smith <ds...@pivotal.io> wrote:
> >
> > > > If we're not opposed to descriptive verbosity, I might prefer
> > > "onServersHostingRegion" more than "onRegion".
> > >
> > > onRegion does not really mean "on the servers hosting region XXX". It
> > only
> > > executes on a subset of the servers, potentially with retries, until it
> > has
> > > covered that entire dataset once. So I think onRegion is more
> > appropriate.
> > >
> > > -Dan
> > >
> > > On Wed, Mar 7, 2018 at 2:38 PM, Patrick Rhomberg <prhomberg@pivotal.io
> >
> > > wrote:
> > >
> > > > +1 for iteration towards better single responsibility design and more
> > > > easily-digestible classes.
> > > >
> > > > Regarding method names, I think that there would be some good utility
> > in
> > > > having "onGroup" methods, as well.
> > > > If we're not opposed to descriptive verbosity, I might prefer
> > > > "onServersHostingRegion" more than "onRegion".
> > > >
> > > > On Wed, Mar 7, 2018 at 1:45 PM, Dan Smith <ds...@pivotal.io> wrote:
> > > >
> > > > > Hi Udo,
> > > > >
> > > > > +1 for making the function service not static and spitting it into
> > > client
> > > > > and server FunctionService objects!
> > > > >
> > > > > We do have Cache and ClientCache right now. So I would recommend
> this
> > > API
> > > > > rather than putting two methods on Cache. Cache is already the the
> > > server
> > > > > side API.
> > > > >
> > > > > Cache {
> > > > > ServerFunctionService getFunctionService()
> > > > > }
> > > > >
> > > > > ClientCache {
> > > > > ClientFunctionService getFunctionService()
> > > > > }
> > > > >
> > > > > If at some point we split the client side API into a separate jar
> the
> > > API
> > > > > shouldn't need to change. If you don't like ClientFunctionService,
> > > maybe
> > > > > o.a.g.cache.client.execute.FunctionService? We would never want
> two
> > > > > different versions of org.apache.geode.function.FunctionService.
> > > People
> > > > > wouldn't be able to test a client and server in the same JVM.
> > > > >
> > > > > Also, you might want to check out this (somewhat stalled) proposal
> -
> > > > > https://cwiki.apache.org/confluence/display/GEODE/
> > > > > Function+Service+Usability+Improvements. We had buy in on this on
> > the
> > > > dev
> > > > > list but have not found cycles to actually do it yet. But maybe now
> > is
> > > > the
> > > > > time?
> > > > >
> > > > > -Dan
> > > > >
> > > > > On Wed, Mar 7, 2018 at 11:18 AM, Udo Kohlmeyer <ud...@apache.org>
> > wrote:
> > > > >
> > > > > > Hi there Apache Dev's,
> > > > > >
> > > > > > Please look at the proposal to improve the FunctionService and
> > remove
> > > > the
> > > > > > static invocation of it from within the Cache.
> > > > > >
> > > > > > https://cwiki.apache.org/confluence/display/GEODE/Function+
> > > > > > Service+Refactor+-+Removal+of+static-ness+and+splitting+of+
> > > > > > client+and+server-side+FunctionService
> > > > > >
> > > > > > --Udo
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
--
-John
john.blum10101 (skype)
Re: FunctionService Proposal
Posted by Galen O'Sullivan <go...@pivotal.io>.
+1 for making the function service not static, and splitting servers from
clients.
Also +1 for Dan's suggestion.
On Wed, Mar 7, 2018 at 2:51 PM, Patrick Rhomberg <pr...@pivotal.io>
wrote:
> I did not know that! And then, yes, onRegion is much better.
>
> On Wed, Mar 7, 2018 at 2:43 PM, Dan Smith <ds...@pivotal.io> wrote:
>
> > > If we're not opposed to descriptive verbosity, I might prefer
> > "onServersHostingRegion" more than "onRegion".
> >
> > onRegion does not really mean "on the servers hosting region XXX". It
> only
> > executes on a subset of the servers, potentially with retries, until it
> has
> > covered that entire dataset once. So I think onRegion is more
> appropriate.
> >
> > -Dan
> >
> > On Wed, Mar 7, 2018 at 2:38 PM, Patrick Rhomberg <pr...@pivotal.io>
> > wrote:
> >
> > > +1 for iteration towards better single responsibility design and more
> > > easily-digestible classes.
> > >
> > > Regarding method names, I think that there would be some good utility
> in
> > > having "onGroup" methods, as well.
> > > If we're not opposed to descriptive verbosity, I might prefer
> > > "onServersHostingRegion" more than "onRegion".
> > >
> > > On Wed, Mar 7, 2018 at 1:45 PM, Dan Smith <ds...@pivotal.io> wrote:
> > >
> > > > Hi Udo,
> > > >
> > > > +1 for making the function service not static and spitting it into
> > client
> > > > and server FunctionService objects!
> > > >
> > > > We do have Cache and ClientCache right now. So I would recommend this
> > API
> > > > rather than putting two methods on Cache. Cache is already the the
> > server
> > > > side API.
> > > >
> > > > Cache {
> > > > ServerFunctionService getFunctionService()
> > > > }
> > > >
> > > > ClientCache {
> > > > ClientFunctionService getFunctionService()
> > > > }
> > > >
> > > > If at some point we split the client side API into a separate jar the
> > API
> > > > shouldn't need to change. If you don't like ClientFunctionService,
> > maybe
> > > > o.a.g.cache.client.execute.FunctionService? We would never want two
> > > > different versions of org.apache.geode.function.FunctionService.
> > People
> > > > wouldn't be able to test a client and server in the same JVM.
> > > >
> > > > Also, you might want to check out this (somewhat stalled) proposal -
> > > > https://cwiki.apache.org/confluence/display/GEODE/
> > > > Function+Service+Usability+Improvements. We had buy in on this on
> the
> > > dev
> > > > list but have not found cycles to actually do it yet. But maybe now
> is
> > > the
> > > > time?
> > > >
> > > > -Dan
> > > >
> > > > On Wed, Mar 7, 2018 at 11:18 AM, Udo Kohlmeyer <ud...@apache.org>
> wrote:
> > > >
> > > > > Hi there Apache Dev's,
> > > > >
> > > > > Please look at the proposal to improve the FunctionService and
> remove
> > > the
> > > > > static invocation of it from within the Cache.
> > > > >
> > > > > https://cwiki.apache.org/confluence/display/GEODE/Function+
> > > > > Service+Refactor+-+Removal+of+static-ness+and+splitting+of+
> > > > > client+and+server-side+FunctionService
> > > > >
> > > > > --Udo
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: FunctionService Proposal
Posted by Patrick Rhomberg <pr...@pivotal.io>.
I did not know that! And then, yes, onRegion is much better.
On Wed, Mar 7, 2018 at 2:43 PM, Dan Smith <ds...@pivotal.io> wrote:
> > If we're not opposed to descriptive verbosity, I might prefer
> "onServersHostingRegion" more than "onRegion".
>
> onRegion does not really mean "on the servers hosting region XXX". It only
> executes on a subset of the servers, potentially with retries, until it has
> covered that entire dataset once. So I think onRegion is more appropriate.
>
> -Dan
>
> On Wed, Mar 7, 2018 at 2:38 PM, Patrick Rhomberg <pr...@pivotal.io>
> wrote:
>
> > +1 for iteration towards better single responsibility design and more
> > easily-digestible classes.
> >
> > Regarding method names, I think that there would be some good utility in
> > having "onGroup" methods, as well.
> > If we're not opposed to descriptive verbosity, I might prefer
> > "onServersHostingRegion" more than "onRegion".
> >
> > On Wed, Mar 7, 2018 at 1:45 PM, Dan Smith <ds...@pivotal.io> wrote:
> >
> > > Hi Udo,
> > >
> > > +1 for making the function service not static and spitting it into
> client
> > > and server FunctionService objects!
> > >
> > > We do have Cache and ClientCache right now. So I would recommend this
> API
> > > rather than putting two methods on Cache. Cache is already the the
> server
> > > side API.
> > >
> > > Cache {
> > > ServerFunctionService getFunctionService()
> > > }
> > >
> > > ClientCache {
> > > ClientFunctionService getFunctionService()
> > > }
> > >
> > > If at some point we split the client side API into a separate jar the
> API
> > > shouldn't need to change. If you don't like ClientFunctionService,
> maybe
> > > o.a.g.cache.client.execute.FunctionService? We would never want two
> > > different versions of org.apache.geode.function.FunctionService.
> People
> > > wouldn't be able to test a client and server in the same JVM.
> > >
> > > Also, you might want to check out this (somewhat stalled) proposal -
> > > https://cwiki.apache.org/confluence/display/GEODE/
> > > Function+Service+Usability+Improvements. We had buy in on this on the
> > dev
> > > list but have not found cycles to actually do it yet. But maybe now is
> > the
> > > time?
> > >
> > > -Dan
> > >
> > > On Wed, Mar 7, 2018 at 11:18 AM, Udo Kohlmeyer <ud...@apache.org> wrote:
> > >
> > > > Hi there Apache Dev's,
> > > >
> > > > Please look at the proposal to improve the FunctionService and remove
> > the
> > > > static invocation of it from within the Cache.
> > > >
> > > > https://cwiki.apache.org/confluence/display/GEODE/Function+
> > > > Service+Refactor+-+Removal+of+static-ness+and+splitting+of+
> > > > client+and+server-side+FunctionService
> > > >
> > > > --Udo
> > > >
> > > >
> > >
> >
>
Re: FunctionService Proposal
Posted by Dan Smith <ds...@pivotal.io>.
> If we're not opposed to descriptive verbosity, I might prefer
"onServersHostingRegion" more than "onRegion".
onRegion does not really mean "on the servers hosting region XXX". It only
executes on a subset of the servers, potentially with retries, until it has
covered that entire dataset once. So I think onRegion is more appropriate.
-Dan
On Wed, Mar 7, 2018 at 2:38 PM, Patrick Rhomberg <pr...@pivotal.io>
wrote:
> +1 for iteration towards better single responsibility design and more
> easily-digestible classes.
>
> Regarding method names, I think that there would be some good utility in
> having "onGroup" methods, as well.
> If we're not opposed to descriptive verbosity, I might prefer
> "onServersHostingRegion" more than "onRegion".
>
> On Wed, Mar 7, 2018 at 1:45 PM, Dan Smith <ds...@pivotal.io> wrote:
>
> > Hi Udo,
> >
> > +1 for making the function service not static and spitting it into client
> > and server FunctionService objects!
> >
> > We do have Cache and ClientCache right now. So I would recommend this API
> > rather than putting two methods on Cache. Cache is already the the server
> > side API.
> >
> > Cache {
> > ServerFunctionService getFunctionService()
> > }
> >
> > ClientCache {
> > ClientFunctionService getFunctionService()
> > }
> >
> > If at some point we split the client side API into a separate jar the API
> > shouldn't need to change. If you don't like ClientFunctionService, maybe
> > o.a.g.cache.client.execute.FunctionService? We would never want two
> > different versions of org.apache.geode.function.FunctionService. People
> > wouldn't be able to test a client and server in the same JVM.
> >
> > Also, you might want to check out this (somewhat stalled) proposal -
> > https://cwiki.apache.org/confluence/display/GEODE/
> > Function+Service+Usability+Improvements. We had buy in on this on the
> dev
> > list but have not found cycles to actually do it yet. But maybe now is
> the
> > time?
> >
> > -Dan
> >
> > On Wed, Mar 7, 2018 at 11:18 AM, Udo Kohlmeyer <ud...@apache.org> wrote:
> >
> > > Hi there Apache Dev's,
> > >
> > > Please look at the proposal to improve the FunctionService and remove
> the
> > > static invocation of it from within the Cache.
> > >
> > > https://cwiki.apache.org/confluence/display/GEODE/Function+
> > > Service+Refactor+-+Removal+of+static-ness+and+splitting+of+
> > > client+and+server-side+FunctionService
> > >
> > > --Udo
> > >
> > >
> >
>
Re: FunctionService Proposal
Posted by Patrick Rhomberg <pr...@pivotal.io>.
+1 for iteration towards better single responsibility design and more
easily-digestible classes.
Regarding method names, I think that there would be some good utility in
having "onGroup" methods, as well.
If we're not opposed to descriptive verbosity, I might prefer
"onServersHostingRegion" more than "onRegion".
On Wed, Mar 7, 2018 at 1:45 PM, Dan Smith <ds...@pivotal.io> wrote:
> Hi Udo,
>
> +1 for making the function service not static and spitting it into client
> and server FunctionService objects!
>
> We do have Cache and ClientCache right now. So I would recommend this API
> rather than putting two methods on Cache. Cache is already the the server
> side API.
>
> Cache {
> ServerFunctionService getFunctionService()
> }
>
> ClientCache {
> ClientFunctionService getFunctionService()
> }
>
> If at some point we split the client side API into a separate jar the API
> shouldn't need to change. If you don't like ClientFunctionService, maybe
> o.a.g.cache.client.execute.FunctionService? We would never want two
> different versions of org.apache.geode.function.FunctionService. People
> wouldn't be able to test a client and server in the same JVM.
>
> Also, you might want to check out this (somewhat stalled) proposal -
> https://cwiki.apache.org/confluence/display/GEODE/
> Function+Service+Usability+Improvements. We had buy in on this on the dev
> list but have not found cycles to actually do it yet. But maybe now is the
> time?
>
> -Dan
>
> On Wed, Mar 7, 2018 at 11:18 AM, Udo Kohlmeyer <ud...@apache.org> wrote:
>
> > Hi there Apache Dev's,
> >
> > Please look at the proposal to improve the FunctionService and remove the
> > static invocation of it from within the Cache.
> >
> > https://cwiki.apache.org/confluence/display/GEODE/Function+
> > Service+Refactor+-+Removal+of+static-ness+and+splitting+of+
> > client+and+server-side+FunctionService
> >
> > --Udo
> >
> >
>
Re: FunctionService Proposal
Posted by Dan Smith <ds...@pivotal.io>.
Hi Udo,
+1 for making the function service not static and spitting it into client
and server FunctionService objects!
We do have Cache and ClientCache right now. So I would recommend this API
rather than putting two methods on Cache. Cache is already the the server
side API.
Cache {
ServerFunctionService getFunctionService()
}
ClientCache {
ClientFunctionService getFunctionService()
}
If at some point we split the client side API into a separate jar the API
shouldn't need to change. If you don't like ClientFunctionService, maybe
o.a.g.cache.client.execute.FunctionService? We would never want two
different versions of org.apache.geode.function.FunctionService. People
wouldn't be able to test a client and server in the same JVM.
Also, you might want to check out this (somewhat stalled) proposal -
https://cwiki.apache.org/confluence/display/GEODE/
Function+Service+Usability+Improvements. We had buy in on this on the dev
list but have not found cycles to actually do it yet. But maybe now is the
time?
-Dan
On Wed, Mar 7, 2018 at 11:18 AM, Udo Kohlmeyer <ud...@apache.org> wrote:
> Hi there Apache Dev's,
>
> Please look at the proposal to improve the FunctionService and remove the
> static invocation of it from within the Cache.
>
> https://cwiki.apache.org/confluence/display/GEODE/Function+
> Service+Refactor+-+Removal+of+static-ness+and+splitting+of+
> client+and+server-side+FunctionService
>
> --Udo
>
>