Table of Contents |
---|
Overview
...
The goal of this proposal is to define clear lines between modules of geode, so that each piece can be tested independently by mocking out the other modules. The goal of is that each module should
- Be mockable - so that that other modules can write tests that don't depend on that module
- Be tested within the module. In other words, if WAN depends on the Region module, I should not need to run any WAN tests to test all of the Code in LocalRegion.
In order to accomplish that goalthese goals, each module needs to have a clear API for the rest of the system to use, and tests within that module that cover all of the features of the API. Each module should have at least it's own package(s), if not be in a separate gradle module. Anything that is not part of the API . for that module should not be accessible outside of the module.
Here's a start at listing the proposed modules and their dependencies. These are proposed dependencies; there currently there many cyclical dependencies. Part of the required changes will breaking some of these cyclic dependencies by replacing hard coded references to other modules with plugins and callbacks that are part of the well defined API for each module.
PlantUML |
---|
@startuml package WAN { } package CacheServer { } package Clienttitle Geode Package Dependency Graph hide members hide circle @startuml title Geode Package Dependency Graph hide members hide circle package Core { } Extensions-->Core Rest-->Management Gfsh-->Management Management-->Core Core-->BasePackages class Management package RegionsExtensions { } package Querying { } package Persistence { } package Statistics { } package Messaging { } package FunctionService { } package Advisors { } package Serialization { } package PDX { } WAN --> CacheServer WAN --> Client WAN --> Regions Regions --> CacheServer Regions --> Client class Redis class MemcacheD class SpringDataGemfire class HttpSession class Hibernate class Spark class Lucene } package BasePackages { class Statistics class Logging class Serialization class Events class DUnit } @enduml |
PlantUML |
---|
@startuml title Geode Core Package Dependency Graph hide members hide circle class WAN CacheServer-->Messaging Client-->Locator ClientSubscriptions-->Regions ClientSubscriptions-->CacheServer ClientSubscriptions-->Client DLock-->Messaging FunctionService-->Messaging FunctionService-->Regions PDX-->Messaging PDX-->Regions Persistence-->Versioning Querying -> Messaging Querying-->Regions Querying-->Indexing Messaging-->Locator Regions-->CacheServer Regions-->Client Regions-->DLock Regions-->Messaging Regions-->OffHeap Regions-->Persistence Regions-->Statistics>ResourceManager AdvisorsRegions-->Messaging>Eviction Regions-->Advisors>Versioning QueryingSnapshots-->Regions Regions->Messaging FunctionService --> Regions FunctionService --> Messaging Messaging -> Serialization PDX -> Serialization PDX -> Regions |
Each module should have at least it's own package(s), if not be in a separate gemfire- gradle module. Anything that is not part of the API for that module should only be package visible. Each module should have a clearly defined API, with tests for that module that cover the API. For example, if there is a method in the Region module to receive an event from the WAN, that should be part of the InternalRegion API and that API should be tested in the Region module.
Below is a high level view of the interfaces for each module and some of the changes that need to be made.
Statistics
Package:internal.statistics
API interfaces/classes: Statistics, StatisticsFactory, StatisticsManager
Snapshots-->FunctionService
ResourceManager-->OffHeap
WAN-->CacheServer
WAN-->Client
WAN-->Messaging
WAN-->Regions
WAN-->Locator
@enduml
|
Packages
Client
Package: internal.cache.client
API interfaces/classes: AbstractOp, ExecutablePool
Required Changes: Operations (The client side code for client-> server messages) for other modules should be moved to their respective packages.
ClientSubscriptions
Package:internal.cache.ha (change this?)
API interface/classes: ?
Required changes: ?
DLock
Package:internal.locks
API interface/classes: InternalDistributedLockService (new interface)
Required changes: ?
Dunit
Package:dunit
API interface/classes: DistributedTestCase,CacheTestCase
Required changes: This code should be moved into it's own gradle module.
Events
Package: internal.cache.event (new package)
API interface/classes: InternalEntryEvent. RegionEntry
Required changes: ? These events are passed everywhere, which is why it would be nice to refactor this code into a separate package that other packages can depend on.
Eviction
Package: internal.cache.lru
API interface/classes: EnableLRU, LRUClockHand (new interface)
Required changes: ?
FunctionService
Package: internal.cache.execute
API interfaces/classes: FunctionService, Execution, Function
Required changes: Region code should be refactored to not have direct dependencies on this package. For example, LocalRegion should not have function execution code in it.
Indexing
Package: query.internal.index
API interface/classes: ?
Required changes: ?
Logging
Package: internal.logging
API interface/classes: ?
Required changes: ?
Locator
Package: distributed.internal.tcpserver
API interface/classes: TcpHandler, ?
Required changes: Move into a separate package. Pull the code out of InternalDistributedSystem (it currently implements StatisticsFactory) into a separate class.Required changes: ?
Messaging
Package:distributed.internal, distributed.internal.advisor
API interfaces/classes: InternalDistributedMember, DM, InternalDistributedSystem, MembershipListener, DistributionAdvisor, DistributionAdvisee
Required Changes: InternalDistributedMember and InternalDistributedSystem should be interfaces, not concrete classes. They should only have the methods that are required by the rest of the system. The concrete classes like DistributionManager, the old InternalDistributedSystem class, etc. should not be referenced outside this package.
TBD - The proposal here is that membership is another module that is hidden behind the messaging layer as far as the rest of the system is concerned. The membership layer has it's own interface that should hide its internals from the messaging layer. Advisors are lumped in here to reduce the complexity of the high level graph.
PlantUML |
---|
@startuml title Messaging Packages hide members hide circle class Messaging package OtherComponents { class Regions class WAN } Advisors |
...
Package: distributed.internal.advisor
--> Messaging
Messaging --> Membership
OtherComponents --> Advisors
OtherComponents --> Messaging
@enduml |
OffHeap
Package: internal.offheap
API interface/classes: ?
Required changes: ?
PDX
Package: pdx.internal
API interface/classes: ?
API interfaces/classes: DistributionAdvisor, DistributionAdviseeRequired changes: ?
Persistence
Package: internal.cache.persistence
...
Required Changes: InternalDiskStore is new interface for DiskStoreImpl.
Querying
Package: query.internal
API interface/classes: QueryService, ?
Required changes: ?
Regions
Package: internal.cache.region
API interfaces/classes: Cache, Region, InternalCache, InternalRegionService, InternalRegion,
Required Changes:
InternalRegion is new interface for LocalRegion. GemfireCacheImpl, RegionService is a new interface that has the functionality from GemfireCacheImpl to manage regions. LocalRegion, etc. should not be used outside this package.
Direct references to other modules, for example LocalRegion.notifyGatewaySender, should be turned into callbacks that other modules plug into the Region interface.
Those callbacks should be tested within the region module.
This is still the big ball of string that needs to get untangled further. We need to split out expiration, conflict detection, GII, transactions, partitioning, etc.
ResourceManager
...
Package: internal.cache.wancontrol
API interfacesinterface/classes:AsyncEventQueue, GatewaySender InternalResourceManager, MemoryEvent, ResourceEvent, ResourceListener
Required Changes: Region code should be refactored to not have direct dependencies on this package. For example, WAN should be notified through a listener installed on the region. The listener interface will be part of the region package.
changes: ? Move rebalancing related classes to the region package?
Serialization
...
Package:internal.cache.executeserialization (new package)
API interfaces/classes: FunctionService, Execution, Function InternalDataSerializer (interface?), DataSerializableFixedID
Required changes: Region code should be refactored to not have direct dependencies on this package. For example, LocalRegion should not have function execution code in it. Changes: ?
Server
Package:internal.cache.tier.sockets (change this?)
...
Required Changes: Commands (the server side code for a client-> server message) for other modules (eg WAN) should be moved to their respective packages and registered with CommandInitializer.
Snapshots
Package: internal.cache.snapshot
API interface/classes: SnapshotService
Required changes:
Statistics
Package:internal.statistics (new package)
API interfaces/classes: Statistics, StatisticsFactory, StatisticsManager
Required changes: Move into a separate package. Pull the code out of InternalDistributedSystem (it currently implements StatisticsFactory) into a separate class
Versioning
Package: internal.
cache.versions
API interface/classes: RegionVersionVector (new interface), VersionTag, VersionStamp
Required changes: ?
WAN
...
Package: internal.cache.clientwan
API interfaces/classes: AbstractOpAsyncEventQueue, ExecutablePoolGatewaySender
Required Changes: Operations (The client side code for client-> server messages) for other modules should be moved to their respective packages. Region code should be refactored to not have direct dependencies on this package. For example, AsyncEventQueues should be notified through a listener installed on the region. The listener interface will be part of the region package.
Questions/Issues
What to do with GemfireCacheImpl,Cache?
The Cache interface currently has dependencies on almost all of the modules of geode because it has methods like getQueryService, getGatewaySenders (WAN). Unfortunately, Cache, InternalCache, or GemfireCacheImpl is used as a context object that also passed to almost all modules of Geode.
We need to rework how we inject dependencies into all of these modules. If we want a context object, it should be something that is generic that does not pull in dependencies on all other services, something likes spring's BeanFactory. But it might be better if the specific dependencies for each module were passed into that module.
Modularity in the public API
We've already started creating a few separate modules at the external level - for example the lucene integration or the auto rebalancer. We need to nail down how these extensions are accessed by the user. One option might be to add a method to Cache like Cache.getService(Class<T> serviceInterface). Maybe we should remove methods like Cache.getQueryService, Cache.getGatewaySenders, etc. in favor of not hardcoding all of the services on Cache?
How to enforce dependencies/interface
Probably not every package list here should be it's own gradle module. How will will enforce the dependencies and the use of the package interfaces?
Package naming scheme
We still have a mix of two different conventions for where to put internal classes. Some things are in gemfire.internal.cache, and some things are in packages like cache.asyncqueue.internal. We should settle on one convention.
Interface and concrete class naming scheme
We seem to have a few conventions that are sometimes used. We should agree on what conventions we want to stick to.
- Having a public interface Cache and an internal interface InternalCache.
- Naming the implementation of an interface *Impl