@Kirk, I think your approach to reduce the number of singleton access is good. I agree and believe that any operational invocation (Function,Command, DistributionMessage,etc...) should provide a mechanism to be able to access all relevant all contextual information required to execute the task (Function,Command, DistributionMessage,etc...)

On 5/24/17 13:41, Kirk Lund wrote:
Thanks for pointing out that DistributionManager is internal -- I forgot
about that. I'm primarily concerned with internal Functions, such as those
for GFSH commands, so maybe an internal version of FunctionContext which
exposes more would be good for those.

On Wed, May 24, 2017 at 11:39 AM, Darrel Schneider <dschnei...@pivotal.io>
wrote:

FunctionContext is an external interface so it can not expose internal
interfaces like DistributionManager.
But it could expose Cache. DistributedSystem is external so you could have
it exposed from FunctionContext but it is already exposed from Cache.
SecurityService is also internal.
Are you thinking that for internal Functions you would cast FunctionContext
to an internal that would then expose these internal classes?



On Thu, May 18, 2017 at 5:13 PM, Kirk Lund <kl...@apache.org> wrote:

I've been digging through our code with close attention to the
singletons.
I believe the majority of singletons in Geode exist for two main reasons:

1) Insufficient context or lack of service lookup for Function,
DistributionMessage and (Client)Command implementations.

2) Poor OO design. This is where you see code in one class invoking
concrete methods on another class outside of its concerns. Many of these
need to be teased apart and replaced with some sort of Observer that
isolates the reaction from the source of the originating event.

Right now my focus is on #1 because that's the area that's currently an
obstacle for me.

Function, DistributionMessage and (Client)Command classes need to have
more
context provided to them (Cache, Security, etc) or they need a better
mechanism to look up these services. Currently these classes reach out to
singletons in order to "get" what they need.

*A) Function*

The main entry-point which injects services into the Function is:

public void execute(FunctionContext<T> context);

The FunctionContext needs to provide the service(s) that any Function
might
require. This could include Cache, DistributionManager and maybe
SecurityService (anything else?).

*B) (Peer-to-peer) DistributionMessage*

The main entry-point which injects services into the DistributionMessage
is:

protected abstract void process(DistributionManager dm);

We could provide multiple arguments or a single new DistributionContext
which then provides DistributionManager and Cache (anything else?).

*C) (Client) Command*

The main entry-point which injects services into the Command is:

public void execute(Message msg, ServerConnection servConn);

ServerConnection is huge but it does already expose Cache. BaseCommand is
the only Command that implements "execute" and it defines a new
entry-point
for injection:

abstract public void cmdExecute(Message msg, ServerConnection servConn,
long start) throws IOException, ClassNotFoundException,
InterruptedException;

We might want to clean that up and define a new CommandContext which
provides the Cache or anything else that the Command may need.

Thoughts or additional ideas?


Reply via email to