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 <dasm...@vmware.com>
Date: Wednesday, October 21, 2020 at 5:14 AM
To: dev@geode.apache.org <dev@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 <jde...@vmware.com>
Sent: Tuesday, October 20, 2020 5:33 AM
To: dev@geode.apache.org <dev@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" <u...@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
