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...@vmware.com> on 2020/10/16 02:25:20 UTC
[DISCUSS] ServiceRegistry RFC
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://cwiki.apache.org/confluence/display/GEODE/ServiceRegistry
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by Udo Kohlmeyer <ud...@vmware.com>.
Hi there Jens,
No this is not intended to replace the Service loading mechanism that is currently there.
This is merely a registry that is to aid the looking up of services (like ClassLoaderService or MetricService) within the code without having to explicitly add that service onto every constructor or method call.
--Udo
From: Jens Deppe <jd...@vmware.com>
Date: Tuesday, October 20, 2020 at 11:33 PM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cudo%40vmware.com%7C592a308778d744660acc08d874f466d2%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637387940332683848%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=5SdJBoPsYXXTQiA0B%2BJDyB38wGnyNhGDa8ePunAKKdQ%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by John Blum <jb...@vmware.com>.
A word of caution here...
I'd like to see us start moving away from "internal" APIs even, as much as possible. Moving away from Singletons is no-brainer, but less obvious, is moving things to proper public APIs and SPIs so that frameworks and tooling can extend Geode in interesting ways. SDG certainly has a need in many cases.
By way of example, I am currently working on Pagination, and I keep running into situations where it would be nice if the same (ANTLR) grammar/parser used to parse OQL was an API SDG could use.
Essentially, every feature and function of Geode should have an interface (API/SPI) that can be extended (e.g. via plugins) in some way to effect function or behavior. This is the primary driver behind the Open/Closed Principle, which would serve Geode well.
$0.02
-j
________________________________
From: Dan Smith <da...@vmware.com>
Sent: Tuesday, October 20, 2020 11:13 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
It might be better to hang things off InternalCache rather than create a new singleton. The cache is already a singleton, but we've been working on removing that by plumbing the cache everywhere. The cache is effectively our context object, and once we've finished removing calls to Cache.getInstance we will have a context object that is not a singleton.
As Jens mentioned, the cache already has the concept of CacheServices that can be retrieved using methods similar to what you listened in this RFC - eg InternalCache.getService(Class clazz).
Can we reuse/generalize/extend this existing service concept for your case?
-Dan
________________________________
From: Jens Deppe <jd...@vmware.com>
Sent: Tuesday, October 20, 2020 5:33 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cjblum%40vmware.com%7C3ee62dcb9d034f24526c08d87523f40c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637388144568538631%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eTRFhI0BECYgeUeimL1tKPI88vuI0SQ4LWWN4ZUWGb8%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by John Blum <jb...@vmware.com>.
FYI..
Spring's ApplicationContext is not necessarily Immutable (e.g. you can register a bean instances after the container has started, or mutate the Environment in some way as necessary by the application for lazy resources/initialization), but the bean configuration is "frozen" (effectively immutable) after the container starts.
________________________________
From: Dan Smith <da...@vmware.com>
Sent: Wednesday, October 21, 2020 2:58 PM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
You mentioned the metrics service - I think that is a good pattern to follow. For metrics the user passes configuration (The user's MeterRegistry) into the CacheFactory, and then the MetricsService hangs off the cache. Places in the code that need metrics fetch the MeterRegistry from the cache.
I don't really see this ServiceRegistryInstance as something that we would want to encourage any developers to use, since it is a singleton. I can see you might need a singleton for the ClassLoaderService because it's too hard to plumb your service into all the places you need it (I can understand that!) and ClassPathLoader is already a singleton. But we don't really want anything else to go in there, because of all the reasons we hate singletons.
For that reason, maybe it would be better just not to have this thing. Instead have a ClassPathServiceSingleton that is clearly a wart we should fix, or hang the ClassPathService off the cache, since we are already trying to find and eliminate places people are calling Cache.getInstance. That way you are not creating a tempting place for developers to stick more services where we really don't want them to.
-Dan
PS - I do like the idea of introducing a ServiceRegistry or something similar (Context, ?). It just should be scoped to a Cache or some shorter lifecycle. And it should be immutable (I believe spring's ApplicationContext might be?), so no adding and removing services from an instance of the ServiceRegistry or Context or whatever.
________________________________
From: Udo Kohlmeyer <ud...@vmware.com>
Sent: Tuesday, October 20, 2020 3:01 PM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Jens, that is a great thought. Lifecycle management would be amazing and I think we can definitely think about that. Right now, my thought of the services that are complimentary to the Cache. i.e the Cache uses them, but they themselves are not dependent on them.
I still see things like HttpService/GeodeRedisService/LuceneService to be modules that are dependent on the cache. Whereas the services that I’m looking at registering in the ServiceRegistry are services that will be used by the Cache and have no direct dependency on the Cache or it’s lifecycle.
e.g MetricsService can life outside of the cache, as there is no dependency on the cache to log a metric. The same would be true for the ClassLoaderService. As the reconnecting of the cache would not have an effect on the classes loaded.
But I like the idea of a lifecycle, but I think that discussion is better left for the broader discussion on lifecycle management of the Cache.
Hope this makes sense.
--Udo
From: Jens Deppe <jd...@vmware.com>
Date: Wednesday, October 21, 2020 at 8:46 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
One aspect this conversation has brought to mind is whether these services will require some kind of lifecycle management? If any of them require an instance of a Cache (and perhaps other components) they will need to be aware of the cache restarting (happens during reconnect).
Is that going to be a problem?
--Jens
On 10/20/20, 1:12 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Dan,
We (Patrick and I) are in violent agreement about adding new singletons to the product. This singleton is merely there to avoid the complete utter re-write of the system to wire in said ServiceRegistry into the cache.
Yes, we are merely replacing 1 singleton with another, but we see it as a step in the right direction that we are replacing the usage with a generic service based implementation, that would allow for the simple “swap-in/out” of class loading (GEODE-8067).
Our inspiration for this ServiceRegistry is from Spring’s ApplicationContext, without all of the cool DI features that Spring would bring. We just needed a single location where one could add services and lookup them up.
We agree, singletons are bad. But hanging everything off InteralCache without a better designed lifecycle for module/component creation would make no sense. THAT is a design that we need to look at to better structure/separate modules/components.
Whilst doing GEODE-8466, and the replacing of ClassPathLoader with an instance-based ClassLoaderService, we found MANY static methods/functions that used the singleton ClassPathLoader, without ANY possibility to be able to wire in a ServiceRegistry instance. In addition, the current singleton is merely reducing the number of files that we need to alter to get this added. We had (from initial GEODE-8466) implementations, learned that passing in a ClassLoaderService instance into each Class/Method that required it, meant we touched 330+ classes. In addition those changes were BEFORE we attempted to replace the static usages of ClassPathLoader with ClassLoaderService.
I imagine that the casuality list of Classes touched would be much higher than 330 if we attempted the same approach with ClassPathLoader. In addition to the crazy effort we’d have to go through to replace the static method/code block usages of ClassPathLoader with instance references to ClassLoaderService.
--Udo
From: Dan Smith <da...@vmware.com>
Date: Wednesday, October 21, 2020 at 5:14 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
It might be better to hang things off InternalCache rather than create a new singleton. The cache is already a singleton, but we've been working on removing that by plumbing the cache everywhere. The cache is effectively our context object, and once we've finished removing calls to Cache.getInstance we will have a context object that is not a singleton.
As Jens mentioned, the cache already has the concept of CacheServices that can be retrieved using methods similar to what you listened in this RFC - eg InternalCache.getService(Class clazz).
Can we reuse/generalize/extend this existing service concept for your case?
-Dan
________________________________
From: Jens Deppe <jd...@vmware.com>
Sent: Tuesday, October 20, 2020 5:33 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cjblum%40vmware.com%7C10e375cd3e554900b34a08d8760c80a2%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637389143353351686%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=zKCt%2FezQrgnAjl3nqcnzeTbkw87qdKCMLdJTgFGxQto%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by Dan Smith <da...@vmware.com>.
You mentioned the metrics service - I think that is a good pattern to follow. For metrics the user passes configuration (The user's MeterRegistry) into the CacheFactory, and then the MetricsService hangs off the cache. Places in the code that need metrics fetch the MeterRegistry from the cache.
I don't really see this ServiceRegistryInstance as something that we would want to encourage any developers to use, since it is a singleton. I can see you might need a singleton for the ClassLoaderService because it's too hard to plumb your service into all the places you need it (I can understand that!) and ClassPathLoader is already a singleton. But we don't really want anything else to go in there, because of all the reasons we hate singletons.
For that reason, maybe it would be better just not to have this thing. Instead have a ClassPathServiceSingleton that is clearly a wart we should fix, or hang the ClassPathService off the cache, since we are already trying to find and eliminate places people are calling Cache.getInstance. That way you are not creating a tempting place for developers to stick more services where we really don't want them to.
-Dan
PS - I do like the idea of introducing a ServiceRegistry or something similar (Context, ?). It just should be scoped to a Cache or some shorter lifecycle. And it should be immutable (I believe spring's ApplicationContext might be?), so no adding and removing services from an instance of the ServiceRegistry or Context or whatever.
________________________________
From: Udo Kohlmeyer <ud...@vmware.com>
Sent: Tuesday, October 20, 2020 3:01 PM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Jens, that is a great thought. Lifecycle management would be amazing and I think we can definitely think about that. Right now, my thought of the services that are complimentary to the Cache. i.e the Cache uses them, but they themselves are not dependent on them.
I still see things like HttpService/GeodeRedisService/LuceneService to be modules that are dependent on the cache. Whereas the services that I’m looking at registering in the ServiceRegistry are services that will be used by the Cache and have no direct dependency on the Cache or it’s lifecycle.
e.g MetricsService can life outside of the cache, as there is no dependency on the cache to log a metric. The same would be true for the ClassLoaderService. As the reconnecting of the cache would not have an effect on the classes loaded.
But I like the idea of a lifecycle, but I think that discussion is better left for the broader discussion on lifecycle management of the Cache.
Hope this makes sense.
--Udo
From: Jens Deppe <jd...@vmware.com>
Date: Wednesday, October 21, 2020 at 8:46 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
One aspect this conversation has brought to mind is whether these services will require some kind of lifecycle management? If any of them require an instance of a Cache (and perhaps other components) they will need to be aware of the cache restarting (happens during reconnect).
Is that going to be a problem?
--Jens
On 10/20/20, 1:12 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Dan,
We (Patrick and I) are in violent agreement about adding new singletons to the product. This singleton is merely there to avoid the complete utter re-write of the system to wire in said ServiceRegistry into the cache.
Yes, we are merely replacing 1 singleton with another, but we see it as a step in the right direction that we are replacing the usage with a generic service based implementation, that would allow for the simple “swap-in/out” of class loading (GEODE-8067).
Our inspiration for this ServiceRegistry is from Spring’s ApplicationContext, without all of the cool DI features that Spring would bring. We just needed a single location where one could add services and lookup them up.
We agree, singletons are bad. But hanging everything off InteralCache without a better designed lifecycle for module/component creation would make no sense. THAT is a design that we need to look at to better structure/separate modules/components.
Whilst doing GEODE-8466, and the replacing of ClassPathLoader with an instance-based ClassLoaderService, we found MANY static methods/functions that used the singleton ClassPathLoader, without ANY possibility to be able to wire in a ServiceRegistry instance. In addition, the current singleton is merely reducing the number of files that we need to alter to get this added. We had (from initial GEODE-8466) implementations, learned that passing in a ClassLoaderService instance into each Class/Method that required it, meant we touched 330+ classes. In addition those changes were BEFORE we attempted to replace the static usages of ClassPathLoader with ClassLoaderService.
I imagine that the casuality list of Classes touched would be much higher than 330 if we attempted the same approach with ClassPathLoader. In addition to the crazy effort we’d have to go through to replace the static method/code block usages of ClassPathLoader with instance references to ClassLoaderService.
--Udo
From: Dan Smith <da...@vmware.com>
Date: Wednesday, October 21, 2020 at 5:14 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
It might be better to hang things off InternalCache rather than create a new singleton. The cache is already a singleton, but we've been working on removing that by plumbing the cache everywhere. The cache is effectively our context object, and once we've finished removing calls to Cache.getInstance we will have a context object that is not a singleton.
As Jens mentioned, the cache already has the concept of CacheServices that can be retrieved using methods similar to what you listened in this RFC - eg InternalCache.getService(Class clazz).
Can we reuse/generalize/extend this existing service concept for your case?
-Dan
________________________________
From: Jens Deppe <jd...@vmware.com>
Sent: Tuesday, October 20, 2020 5:33 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cdasmith%40vmware.com%7Cfd01791904894aa6b3bf08d87543c0c2%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637388281158007225%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NXRaew0%2F6fOPLK96k1yMx7ukkD57UOXsPdW9EYs5SAY%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by Udo Kohlmeyer <ud...@vmware.com>.
Jens, that is a great thought. Lifecycle management would be amazing and I think we can definitely think about that. Right now, my thought of the services that are complimentary to the Cache. i.e the Cache uses them, but they themselves are not dependent on them.
I still see things like HttpService/GeodeRedisService/LuceneService to be modules that are dependent on the cache. Whereas the services that I’m looking at registering in the ServiceRegistry are services that will be used by the Cache and have no direct dependency on the Cache or it’s lifecycle.
e.g MetricsService can life outside of the cache, as there is no dependency on the cache to log a metric. The same would be true for the ClassLoaderService. As the reconnecting of the cache would not have an effect on the classes loaded.
But I like the idea of a lifecycle, but I think that discussion is better left for the broader discussion on lifecycle management of the Cache.
Hope this makes sense.
--Udo
From: Jens Deppe <jd...@vmware.com>
Date: Wednesday, October 21, 2020 at 8:46 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
One aspect this conversation has brought to mind is whether these services will require some kind of lifecycle management? If any of them require an instance of a Cache (and perhaps other components) they will need to be aware of the cache restarting (happens during reconnect).
Is that going to be a problem?
--Jens
On 10/20/20, 1:12 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Dan,
We (Patrick and I) are in violent agreement about adding new singletons to the product. This singleton is merely there to avoid the complete utter re-write of the system to wire in said ServiceRegistry into the cache.
Yes, we are merely replacing 1 singleton with another, but we see it as a step in the right direction that we are replacing the usage with a generic service based implementation, that would allow for the simple “swap-in/out” of class loading (GEODE-8067).
Our inspiration for this ServiceRegistry is from Spring’s ApplicationContext, without all of the cool DI features that Spring would bring. We just needed a single location where one could add services and lookup them up.
We agree, singletons are bad. But hanging everything off InteralCache without a better designed lifecycle for module/component creation would make no sense. THAT is a design that we need to look at to better structure/separate modules/components.
Whilst doing GEODE-8466, and the replacing of ClassPathLoader with an instance-based ClassLoaderService, we found MANY static methods/functions that used the singleton ClassPathLoader, without ANY possibility to be able to wire in a ServiceRegistry instance. In addition, the current singleton is merely reducing the number of files that we need to alter to get this added. We had (from initial GEODE-8466) implementations, learned that passing in a ClassLoaderService instance into each Class/Method that required it, meant we touched 330+ classes. In addition those changes were BEFORE we attempted to replace the static usages of ClassPathLoader with ClassLoaderService.
I imagine that the casuality list of Classes touched would be much higher than 330 if we attempted the same approach with ClassPathLoader. In addition to the crazy effort we’d have to go through to replace the static method/code block usages of ClassPathLoader with instance references to ClassLoaderService.
--Udo
From: Dan Smith <da...@vmware.com>
Date: Wednesday, October 21, 2020 at 5:14 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
It might be better to hang things off InternalCache rather than create a new singleton. The cache is already a singleton, but we've been working on removing that by plumbing the cache everywhere. The cache is effectively our context object, and once we've finished removing calls to Cache.getInstance we will have a context object that is not a singleton.
As Jens mentioned, the cache already has the concept of CacheServices that can be retrieved using methods similar to what you listened in this RFC - eg InternalCache.getService(Class clazz).
Can we reuse/generalize/extend this existing service concept for your case?
-Dan
________________________________
From: Jens Deppe <jd...@vmware.com>
Sent: Tuesday, October 20, 2020 5:33 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cudo%40vmware.com%7Ced59088e5a8047dbf70e08d875418e08%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637388271700062818%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=AeujM%2BnSmky5T51smRtQZ%2FHX6OJw9T8%2FgBiAekNjy5M%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by Jens Deppe <jd...@vmware.com>.
One aspect this conversation has brought to mind is whether these services will require some kind of lifecycle management? If any of them require an instance of a Cache (and perhaps other components) they will need to be aware of the cache restarting (happens during reconnect).
Is that going to be a problem?
--Jens
On 10/20/20, 1:12 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Dan,
We (Patrick and I) are in violent agreement about adding new singletons to the product. This singleton is merely there to avoid the complete utter re-write of the system to wire in said ServiceRegistry into the cache.
Yes, we are merely replacing 1 singleton with another, but we see it as a step in the right direction that we are replacing the usage with a generic service based implementation, that would allow for the simple “swap-in/out” of class loading (GEODE-8067).
Our inspiration for this ServiceRegistry is from Spring’s ApplicationContext, without all of the cool DI features that Spring would bring. We just needed a single location where one could add services and lookup them up.
We agree, singletons are bad. But hanging everything off InteralCache without a better designed lifecycle for module/component creation would make no sense. THAT is a design that we need to look at to better structure/separate modules/components.
Whilst doing GEODE-8466, and the replacing of ClassPathLoader with an instance-based ClassLoaderService, we found MANY static methods/functions that used the singleton ClassPathLoader, without ANY possibility to be able to wire in a ServiceRegistry instance. In addition, the current singleton is merely reducing the number of files that we need to alter to get this added. We had (from initial GEODE-8466) implementations, learned that passing in a ClassLoaderService instance into each Class/Method that required it, meant we touched 330+ classes. In addition those changes were BEFORE we attempted to replace the static usages of ClassPathLoader with ClassLoaderService.
I imagine that the casuality list of Classes touched would be much higher than 330 if we attempted the same approach with ClassPathLoader. In addition to the crazy effort we’d have to go through to replace the static method/code block usages of ClassPathLoader with instance references to ClassLoaderService.
--Udo
From: Dan Smith <da...@vmware.com>
Date: Wednesday, October 21, 2020 at 5:14 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
It might be better to hang things off InternalCache rather than create a new singleton. The cache is already a singleton, but we've been working on removing that by plumbing the cache everywhere. The cache is effectively our context object, and once we've finished removing calls to Cache.getInstance we will have a context object that is not a singleton.
As Jens mentioned, the cache already has the concept of CacheServices that can be retrieved using methods similar to what you listened in this RFC - eg InternalCache.getService(Class clazz).
Can we reuse/generalize/extend this existing service concept for your case?
-Dan
________________________________
From: Jens Deppe <jd...@vmware.com>
Sent: Tuesday, October 20, 2020 5:33 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cjdeppe%40vmware.com%7Cb2a0e69779a74f141b7108d87534841b%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637388215707347503%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=5Z6jcDR31xycOX9pBDRBl%2B7aBkHloSI6De6XjGY3lO0%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by Udo Kohlmeyer <ud...@vmware.com>.
Hi there Dan,
We (Patrick and I) are in violent agreement about adding new singletons to the product. This singleton is merely there to avoid the complete utter re-write of the system to wire in said ServiceRegistry into the cache.
Yes, we are merely replacing 1 singleton with another, but we see it as a step in the right direction that we are replacing the usage with a generic service based implementation, that would allow for the simple “swap-in/out” of class loading (GEODE-8067).
Our inspiration for this ServiceRegistry is from Spring’s ApplicationContext, without all of the cool DI features that Spring would bring. We just needed a single location where one could add services and lookup them up.
We agree, singletons are bad. But hanging everything off InteralCache without a better designed lifecycle for module/component creation would make no sense. THAT is a design that we need to look at to better structure/separate modules/components.
Whilst doing GEODE-8466, and the replacing of ClassPathLoader with an instance-based ClassLoaderService, we found MANY static methods/functions that used the singleton ClassPathLoader, without ANY possibility to be able to wire in a ServiceRegistry instance. In addition, the current singleton is merely reducing the number of files that we need to alter to get this added. We had (from initial GEODE-8466) implementations, learned that passing in a ClassLoaderService instance into each Class/Method that required it, meant we touched 330+ classes. In addition those changes were BEFORE we attempted to replace the static usages of ClassPathLoader with ClassLoaderService.
I imagine that the casuality list of Classes touched would be much higher than 330 if we attempted the same approach with ClassPathLoader. In addition to the crazy effort we’d have to go through to replace the static method/code block usages of ClassPathLoader with instance references to ClassLoaderService.
--Udo
From: Dan Smith <da...@vmware.com>
Date: Wednesday, October 21, 2020 at 5:14 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
It might be better to hang things off InternalCache rather than create a new singleton. The cache is already a singleton, but we've been working on removing that by plumbing the cache everywhere. The cache is effectively our context object, and once we've finished removing calls to Cache.getInstance we will have a context object that is not a singleton.
As Jens mentioned, the cache already has the concept of CacheServices that can be retrieved using methods similar to what you listened in this RFC - eg InternalCache.getService(Class clazz).
Can we reuse/generalize/extend this existing service concept for your case?
-Dan
________________________________
From: Jens Deppe <jd...@vmware.com>
Sent: Tuesday, October 20, 2020 5:33 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cudo%40vmware.com%7C12fcf6167a14483db58508d87523f5ca%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637388144610285623%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=zUM18Gb6ta1rwJm3shFRXyXM0E1uWEZVD2ifEz2LW%2BU%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by Dan Smith <da...@vmware.com>.
It might be better to hang things off InternalCache rather than create a new singleton. The cache is already a singleton, but we've been working on removing that by plumbing the cache everywhere. The cache is effectively our context object, and once we've finished removing calls to Cache.getInstance we will have a context object that is not a singleton.
As Jens mentioned, the cache already has the concept of CacheServices that can be retrieved using methods similar to what you listened in this RFC - eg InternalCache.getService(Class clazz).
Can we reuse/generalize/extend this existing service concept for your case?
-Dan
________________________________
From: Jens Deppe <jd...@vmware.com>
Sent: Tuesday, October 20, 2020 5:33 AM
To: dev@geode.apache.org <de...@geode.apache.org>
Subject: Re: [DISCUSS] ServiceRegistry RFC
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cdasmith%40vmware.com%7C1eb01e77f0694a3b830e08d874f46868%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637387940356374371%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=o7G%2FGj0tkLvBhf4eSCBLA9GzSK0p%2BsjP4fplWJJNNjs%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo
Re: [DISCUSS] ServiceRegistry RFC
Posted by Jens Deppe <jd...@vmware.com>.
Hi Udo,
Is the intention of this to replace (and extend) what we're doing with the traditional service loading mechanism today? i.e. how we're loading everything that extends CacheService (for example HttpService, LuceneService, GeodeRedisService, etc.).
Thanks
--jens
On 10/15/20, 7:25 PM, "Udo Kohlmeyer" <ud...@vmware.com> wrote:
Hi there Apache Geode Devs.
Please find attached an RFC for the introduction of a ServiceRegistry into Apache Geode.
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FGEODE%2FServiceRegistry&data=04%7C01%7Cjdeppe%40vmware.com%7C2992b6fe368a40379f3708d8717ac518%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637384119403978572%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=3WLpKRKqQecQuLfoX7M18OCw4P8%2Brv0xNLqoX%2FUJ0eo%3D&reserved=0
Please add all comments to the RFC to this email for tracking and discussion.
--Udo