Skip to content

Commit

Permalink
GEODE-7177: Logging as a submodule (apache#4129)
Browse files Browse the repository at this point in the history
* GEODE-7177: Extract LogService to a separate submodule

- Extracting LogService and it's dependencies to a separate geode-logging
submodule so that other modules not in geode-core can still use LogService to
get a Logger.

- Removing the use of the logging SPI from LogService
LogService had a dependency on the logging SPI in geode-core. However, this
dependency was unecessary - the LoggingProvider loaded statically in this class
never had `configure` called on it. In addition, the getLogger methods of this
class are merely creating FastLoggers which can live inside geode-logging and
do not need to be pluggable.

- Moved executors that are not dependent on geode-core to geode-logging and
renamed the remaining executors to be CoreLoggingExecutors.

- Refactor the marker interface from Loggable to EntriesCollection

- EntriesCollection put back into internal.cache and AbstractRegion now implements the marker
- Formerly known as Loggable renamed to LogWithToString

- Remove marker interface called EntriesCollection completely, replace with LogWithToString.
  • Loading branch information
Ernie Burghardt authored Oct 10, 2019
1 parent 0be31dd commit 5981a13
Show file tree
Hide file tree
Showing 91 changed files with 486 additions and 384 deletions.
6 changes: 6 additions & 0 deletions boms/geode-all-bom/src/test/resources/expected-pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,12 @@
<version>1.11.0-SNAPSHOT</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.apache.geode</groupId>
<artifactId>geode-logging</artifactId>
<version>1.11.0-SNAPSHOT</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.apache.geode</groupId>
<artifactId>geode-lucene</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ SESSION_JAR="${LIB_DIR}/geode-modules-session-${VERSION}.jar"

declare -a OTHER_JARS
OTHER_JARS=(${GEODE}/lib/geode-core-${VERSION}.jar \
${GEODE}/lib/geode-logging-${VERSION}.jar \
${GEODE}/lib/geode-serialization-${VERSION}.jar \
${GEODE}/lib/geode-common-${VERSION}.jar \
${GEODE}/lib/geode-log4j-${VERSION}.jar \
Expand Down
1 change: 1 addition & 0 deletions extensions/geode-modules-session-internal/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ dependencies {
compile(platform(project(':boms:geode-all-bom')))
compile(project(':extensions:geode-modules'))
compile(project(':geode-core'))
implementation(project(':geode-logging'))

compile('javax.servlet:javax.servlet-api')
compile('mx4j:mx4j')
Expand Down
1 change: 1 addition & 0 deletions extensions/geode-modules-session/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dependencies {
integrationTestCompile(project(':geode-dunit')) {
exclude module: 'geode-core'
}
integrationTestImplementation(project(':geode-logging'))

compile('javax.servlet:javax.servlet-api')
compile('org.apache.tomcat:servlet-api:' + DependencyConstraints.get('tomcat6.version'))
Expand Down
2 changes: 2 additions & 0 deletions extensions/geode-modules-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ dependencies {
exclude group: 'org.apache.tomcat'
}
compile(project(':geode-core'))
implementation(project(':geode-logging'))

compile(project(':geode-junit')) {
exclude module: 'geode-core'
}
Expand Down
1 change: 1 addition & 0 deletions extensions/geode-modules-tomcat8/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ dependencies {
compile(platform(project(':boms:geode-all-bom')))
distributedTestCompile('junit:junit')
compile('mx4j:mx4j')
distributedTestImplementation(project(':geode-logging'))
distributedTestCompile('org.httpunit:httpunit')
distributedTestCompile('org.apache.tomcat:tomcat-jaspic-api:' + DependencyConstraints.get('tomcat8.version'))
compile(project(':geode-core'))
Expand Down
1 change: 1 addition & 0 deletions extensions/geode-modules/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ evaluationDependsOn(":geode-core")

dependencies {
compile(platform(project(':boms:geode-all-bom')))
implementation(project(':geode-logging'))
implementation(project(':geode-serialization'))
compile(project(':geode-core'))
integrationTestCompile(project(':extensions:geode-modules-test')) {
Expand Down
6 changes: 5 additions & 1 deletion geode-assembly/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def dependentProjectNames = [
':geode-core',
':geode-cq',
':geode-log4j',
':geode-logging',
':geode-lucene',
':geode-memcached',
':geode-old-client-support',
Expand Down Expand Up @@ -162,6 +163,7 @@ dependencies {
geodeArchives project(':geode-core')
geodeArchives project(':geode-cq')
geodeArchives project(':geode-log4j')
geodeArchives project(':geode-logging')
geodeArchives project(':geode-lucene')
geodeArchives project(':geode-management')
geodeArchives project(':geode-memcached')
Expand Down Expand Up @@ -211,11 +213,13 @@ dependencies {
integrationTestCompile(project(':geode-assembly:geode-assembly-test'))
integrationTestCompile('org.apache.httpcomponents:httpclient')
integrationTestCompile('javax.annotation:javax.annotation-api')
integrationTestImplementation(project(':geode-logging'))


integrationTestRuntime('io.swagger:swagger-annotations')


distributedTestImplementation(project(':geode-logging'))
distributedTestImplementation(project(':geode-serialization'))
distributedTestCompile(project(':geode-core'))
distributedTestCompile(project(':geode-log4j')) {
Expand Down Expand Up @@ -269,7 +273,7 @@ dependencies {
uiTestRuntime(project(':geode-core'))
uiTestRuntime('org.seleniumhq.selenium:selenium-chrome-driver')


upgradeTestImplementation project(':geode-logging')
upgradeTestImplementation project(':geode-serialization')
upgradeTestCompile(project(':geode-core'))
upgradeTestCompile(project(':geode-dunit')) {
Expand Down
1 change: 1 addition & 0 deletions geode-assembly/geode-assembly-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ dependencies {
compileOnly(project(':extensions:geode-modules-test'))
compileOnly(project(':geode-core'))
compileOnly(project(':geode-pulse'))
implementation(project(':geode-logging'))
compileOnly('com.fasterxml.jackson.core:jackson-databind')
compileOnly('commons-io:commons-io')
compileOnly('junit:junit')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public String jarSkipPropertyName() {
*/
private static final String[] tomcatRequiredJars =
{"antlr", "commons-io", "commons-lang", "commons-validator", "fastutil", "geode-common",
"geode-core", "geode-log4j", "geode-management", "geode-serialization",
"geode-core", "geode-log4j", "geode-logging", "geode-management", "geode-serialization",
"javax.transaction-api", "jgroups", "log4j-api", "log4j-core", "log4j-jul", "micrometer",
"shiro-core", "jetty-server", "jetty-util", "jetty-http", "jetty-io"};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,7 @@ lib/geode-dependencies.jar
lib/geode-http-service-0.0.0.jar
lib/geode-jca-0.0.0.rar
lib/geode-log4j-0.0.0.jar
lib/geode-logging-0.0.0.jar
lib/geode-lucene-0.0.0.jar
lib/geode-management-0.0.0.jar
lib/geode-memcached-0.0.0.jar
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ geode-core-0.0.0.jar
geode-cq-0.0.0.jar
geode-http-service-0.0.0.jar
geode-log4j-0.0.0.jar
geode-logging-0.0.0.jar
geode-lucene-0.0.0.jar
geode-management-0.0.0.jar
geode-memcached-0.0.0.jar
Expand Down
1 change: 1 addition & 0 deletions geode-connectors/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ task downloadJdbcJars(type:Copy) {

dependencies {
compile(platform(project(':boms:geode-all-bom')))
implementation(project(':geode-logging'))
implementation(project(':geode-serialization'))
compile(project(':geode-core'))
testCompile(project(':geode-junit')) {
Expand Down
5 changes: 5 additions & 0 deletions geode-connectors/src/test/resources/expected-pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@
<artifactId>rmiio</artifactId>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.apache.geode</groupId>
<artifactId>geode-logging</artifactId>
<scope>runtime</scope>
</dependency>
<dependency>
<groupId>org.apache.geode</groupId>
<artifactId>geode-serialization</artifactId>
Expand Down
1 change: 1 addition & 0 deletions geode-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ dependencies {

//Geode-common has annotations and other pieces used geode-core
api(project(':geode-common'))
implementation(project(':geode-logging'))
implementation(project(':geode-unsafe'))
implementation(project(':geode-serialization'))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@
import org.apache.geode.internal.cache.PoolManagerImpl;
import org.apache.geode.internal.cache.PoolStats;
import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
import org.apache.geode.internal.logging.CoreLoggingExecutors;
import org.apache.geode.internal.logging.InternalLogWriter;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.LoggingExecutors;
import org.apache.geode.internal.monitoring.ThreadsMonitoring;
import org.apache.geode.internal.statistics.DummyStatisticsFactory;

Expand Down Expand Up @@ -320,7 +320,7 @@ private void start() {
}

final String timerName = "poolTimer-" + getName() + "-";
backgroundProcessor = LoggingExecutors.newScheduledThreadPool(timerName,
backgroundProcessor = CoreLoggingExecutors.newScheduledThreadPool(timerName,
BACKGROUND_TASK_POOL_SIZE, BACKGROUND_TASK_POOL_KEEP_ALIVE, threadMonitoring);
source.start(this);
connectionFactory.start(backgroundProcessor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
import org.apache.geode.SystemFailure;
import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
import org.apache.geode.distributed.internal.membership.gms.messages.ViewAckMessage;
import org.apache.geode.internal.logging.CoreLoggingExecutors;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.LoggingExecutors;
import org.apache.geode.internal.logging.log4j.LogMarker;
import org.apache.geode.internal.monitoring.ThreadsMonitoring;
import org.apache.geode.internal.monitoring.ThreadsMonitoringImpl;
Expand Down Expand Up @@ -220,25 +220,25 @@ public class ClusterOperationExecutors implements OperationExecutors {
stats.getSerialQueueHelper());
poolQueue = serialQueue;
}
serialThread = LoggingExecutors.newSerialThreadPool("Serial Message Processor",
serialThread = CoreLoggingExecutors.newSerialThreadPool("Serial Message Processor",
thread -> stats.incSerialThreadStarts(),
this::doSerialThread, stats.getSerialProcessorHelper(),
threadMonitor, poolQueue);

}

viewThread =
LoggingExecutors.newSerialThreadPoolWithUnlimitedFeed("View Message Processor",
CoreLoggingExecutors.newSerialThreadPoolWithUnlimitedFeed("View Message Processor",
thread -> stats.incViewThreadStarts(), this::doViewThread,
stats.getViewProcessorHelper(), threadMonitor);

threadPool =
LoggingExecutors.newThreadPoolWithFeedStatistics("Pooled Message Processor ",
CoreLoggingExecutors.newThreadPoolWithFeedStatistics("Pooled Message Processor ",
thread -> stats.incProcessingThreadStarts(), this::doProcessingThread,
MAX_THREADS, stats.getNormalPoolHelper(), threadMonitor,
INCOMING_QUEUE_LIMIT, stats.getOverflowQueueHelper());

highPriorityPool = LoggingExecutors.newThreadPoolWithFeedStatistics(
highPriorityPool = CoreLoggingExecutors.newThreadPoolWithFeedStatistics(
"Pooled High Priority Message Processor ",
thread -> stats.incHighPriorityThreadStarts(), this::doHighPriorityThread,
MAX_THREADS, stats.getHighPriorityPoolHelper(), threadMonitor,
Expand All @@ -252,41 +252,43 @@ public class ClusterOperationExecutors implements OperationExecutors {
} else {
poolQueue = new OverflowQueueWithDMStats<>(stats.getWaitingQueueHelper());
}
waitingPool = LoggingExecutors.newThreadPool("Pooled Waiting Message Processor ",
waitingPool = CoreLoggingExecutors.newThreadPool("Pooled Waiting Message Processor ",
thread -> stats.incWaitingThreadStarts(), this::doWaitingThread,
MAX_WAITING_THREADS, stats.getWaitingPoolHelper(), threadMonitor, poolQueue);
}

// should this pool using the waiting pool stats?
prMetaDataCleanupThreadPool =
LoggingExecutors.newThreadPoolWithFeedStatistics("PrMetaData cleanup Message Processor ",
CoreLoggingExecutors.newThreadPoolWithFeedStatistics(
"PrMetaData cleanup Message Processor ",
thread -> stats.incWaitingThreadStarts(), this::doWaitingThread,
MAX_PR_META_DATA_CLEANUP_THREADS, stats.getWaitingPoolHelper(), threadMonitor,
0, stats.getWaitingQueueHelper());

if (MAX_PR_THREADS > 1) {
partitionedRegionPool =
LoggingExecutors.newThreadPoolWithFeedStatistics("PartitionedRegion Message Processor",
CoreLoggingExecutors.newThreadPoolWithFeedStatistics(
"PartitionedRegion Message Processor",
thread -> stats.incPartitionedRegionThreadStarts(), this::doPartitionRegionThread,
MAX_PR_THREADS, stats.getPartitionedRegionPoolHelper(), threadMonitor,
INCOMING_QUEUE_LIMIT, stats.getPartitionedRegionQueueHelper());
} else {
partitionedRegionThread = LoggingExecutors.newSerialThreadPoolWithFeedStatistics(
partitionedRegionThread = CoreLoggingExecutors.newSerialThreadPoolWithFeedStatistics(
"PartitionedRegion Message Processor",
thread -> stats.incPartitionedRegionThreadStarts(), this::doPartitionRegionThread,
stats.getPartitionedRegionPoolHelper(), threadMonitor,
INCOMING_QUEUE_LIMIT, stats.getPartitionedRegionQueueHelper());
}
if (MAX_FE_THREADS > 1) {
functionExecutionPool =
LoggingExecutors.newFunctionThreadPoolWithFeedStatistics(
CoreLoggingExecutors.newFunctionThreadPoolWithFeedStatistics(
FUNCTION_EXECUTION_PROCESSOR_THREAD_PREFIX,
thread -> stats.incFunctionExecutionThreadStarts(), this::doFunctionExecutionThread,
MAX_FE_THREADS, stats.getFunctionExecutionPoolHelper(), threadMonitor,
INCOMING_QUEUE_LIMIT, stats.getFunctionExecutionQueueHelper());
} else {
functionExecutionThread =
LoggingExecutors.newSerialThreadPoolWithFeedStatistics(
CoreLoggingExecutors.newSerialThreadPoolWithFeedStatistics(
FUNCTION_EXECUTION_PROCESSOR_THREAD_PREFIX,
thread -> stats.incFunctionExecutionThreadStarts(), this::doFunctionExecutionThread,
stats.getFunctionExecutionPoolHelper(), threadMonitor,
Expand Down Expand Up @@ -847,7 +849,7 @@ private ExecutorService createSerialExecutor(final Integer id) {

serialQueuedMap.put(id, poolQueue);

return LoggingExecutors.newSerialThreadPool("Pooled Serial Message Processor" + id + "-",
return CoreLoggingExecutors.newSerialThreadPool("Pooled Serial Message Processor" + id + "-",
thread -> stats.incSerialPooledThreadStarts(), this::doSerialPooledThread,
stats.getSerialPooledProcessorHelper(), threadMonitoring, poolQueue);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.LogWriterFactory;
import org.apache.geode.internal.logging.LoggingThread;
import org.apache.geode.internal.logging.LoggingUncaughtExceptionHandler;
import org.apache.geode.internal.net.SocketCreatorFactory;
import org.apache.geode.internal.offheap.MemoryAllocator;
import org.apache.geode.internal.offheap.OffHeapStorage;
Expand Down Expand Up @@ -537,6 +538,8 @@ private InternalDistributedSystem(ConnectionConfig config,
StatisticsManagerFactory statisticsManagerFactory) {
alertingSession = AlertingSession.create();
alertingService = new InternalAlertingServiceFactory().create();
LoggingUncaughtExceptionHandler
.setFailureSetter(error -> SystemFailure.setFailure((VirtualMachineError) error));
loggingSession = LoggingSession.create();
originalConfig = config.distributionConfig();
isReconnectingDS = config.isReconnecting();
Expand Down Expand Up @@ -2186,7 +2189,7 @@ public Properties getSecurityProperties() {
// .uncleanShutdown("VM is exiting", null);
// }
}
});
}, false);
Runtime.getRuntime().addShutdownHook(tmp_shutdownHook);
}
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@
import org.apache.geode.internal.cache.client.protocol.exception.ServiceVersionNotFoundException;
import org.apache.geode.internal.cache.tier.CommunicationMode;
import org.apache.geode.internal.cache.tier.sockets.Handshake;
import org.apache.geode.internal.logging.CoreLoggingExecutors;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.LoggingExecutors;
import org.apache.geode.internal.logging.LoggingThread;
import org.apache.geode.internal.net.SocketCreator;
import org.apache.geode.internal.net.SocketCreatorFactory;
Expand Down Expand Up @@ -187,7 +187,7 @@ protected SocketCreator getSocketCreator() {
}

private static ExecutorService createExecutor(PoolStatHelper poolHelper) {
return LoggingExecutors.newThreadPoolWithSynchronousFeed("locator request thread ",
return CoreLoggingExecutors.newThreadPoolWithSynchronousFeed("locator request thread ",
MAX_POOL_SIZE, poolHelper, POOL_IDLE_TIMEOUT, new ThreadPoolExecutor.CallerRunsPolicy());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
import org.apache.geode.internal.cache.extension.SimpleExtensionPoint;
import org.apache.geode.internal.cache.snapshot.RegionSnapshotServiceImpl;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.LogWithToString;
import org.apache.geode.internal.serialization.DataSerializableFixedID;
import org.apache.geode.internal.serialization.DeserializationContext;
import org.apache.geode.internal.serialization.SerializationContext;
Expand All @@ -106,7 +107,7 @@
*/
@SuppressWarnings("deprecation")
public abstract class AbstractRegion implements InternalRegion, AttributesMutator, CacheStatistics,
DataSerializableFixedID, Extensible<Region<?, ?>>, EvictableRegion {
DataSerializableFixedID, Extensible<Region<?, ?>>, EvictableRegion, LogWithToString {

private static final Logger logger = LogService.getLogger();
private final ReentrantReadWriteLock readWriteLockForCacheLoader = new ReentrantReadWriteLock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@
import org.apache.geode.cache.Region;
import org.apache.geode.internal.cache.LocalRegion.IteratorType;
import org.apache.geode.internal.cache.entries.AbstractRegionEntry;
import org.apache.geode.internal.logging.LogWithToString;

/** Set view of entries */
public class EntriesSet extends AbstractSet implements EntriesCollection {
public class EntriesSet extends AbstractSet implements LogWithToString {

final LocalRegion topRegion;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
import org.apache.geode.distributed.internal.DistributionConfig;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
import org.apache.geode.internal.SystemTimer;
import org.apache.geode.internal.logging.CoreLoggingExecutors;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.LoggingExecutors;
import org.apache.geode.internal.tcp.ConnectionTable;

/**
Expand All @@ -52,7 +52,7 @@ public abstract class ExpiryTask extends SystemTimer.SystemTimerTask {
// default to inline expiry to fix bug 37115
int nThreads = Integer.getInteger(DistributionConfig.GEMFIRE_PREFIX + "EXPIRY_THREADS", 0);
if (nThreads > 0) {
executor = LoggingExecutors.newThreadPoolWithSynchronousFeed("Expiry ",
executor = CoreLoggingExecutors.newThreadPoolWithSynchronousFeed("Expiry ",
(Runnable command) -> doExpiryThread(command),
nThreads);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@
import org.apache.geode.internal.jndi.JNDIInvoker;
import org.apache.geode.internal.jta.TransactionManagerImpl;
import org.apache.geode.internal.lang.ThrowableUtils;
import org.apache.geode.internal.logging.CoreLoggingExecutors;
import org.apache.geode.internal.logging.InternalLogWriter;
import org.apache.geode.internal.logging.LogService;
import org.apache.geode.internal.logging.LoggingExecutors;
Expand Down Expand Up @@ -855,7 +856,7 @@ public static GemFireCacheImpl getForPdx(String reason) {
persistentMemberManager = new PersistentMemberManager();

if (useAsyncEventListeners) {
eventThreadPool = LoggingExecutors.newThreadPoolWithFixedFeed("Message Event Thread",
eventThreadPool = CoreLoggingExecutors.newThreadPoolWithFixedFeed("Message Event Thread",
command -> {
ConnectionTable.threadWantsSharedResources();
command.run();
Expand Down
Loading

0 comments on commit 5981a13

Please sign in to comment.