You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2017/01/23 18:41:41 UTC
[1/2] geode git commit: GEODE-2240: prevent unsafe concurrent access
to ArrayList [Forced Update!]
Repository: geode
Updated Branches:
refs/heads/feature/GEODE-2297-3 31a81b351 -> fcf9085f1 (forced update)
GEODE-2240: prevent unsafe concurrent access to ArrayList
Fixed deadlock by adding expiredTombstonesLock.
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/4d587789
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/4d587789
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/4d587789
Branch: refs/heads/feature/GEODE-2297-3
Commit: 4d58778985b832a7899056a06fb0de0f2995f55e
Parents: 85d45e8
Author: Darrel Schneider <ds...@pivotal.io>
Authored: Tue Jan 3 17:00:07 2017 -0800
Committer: Darrel Schneider <ds...@pivotal.io>
Committed: Fri Jan 20 14:15:34 2017 -0800
----------------------------------------------------------------------
.../geode/internal/cache/TombstoneService.java | 22 ++++++++++++--------
1 file changed, 13 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/4d587789/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java b/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
index 2840134..ca682bc 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
@@ -406,10 +406,10 @@ public class TombstoneService {
*/
private final ExecutorService executor;
/**
- * tombstones that have expired and are awaiting batch removal. This variable is only accessed
- * by the sweeper thread and so is not guarded
+ * tombstones that have expired and are awaiting batch removal.
*/
private final List<Tombstone> expiredTombstones;
+ private final Object expiredTombstonesLock = new Object();
/**
* Force batch expiration
@@ -470,7 +470,7 @@ public class TombstoneService {
protected boolean removeExpiredIf(Predicate<Tombstone> predicate) {
boolean result = false;
long removalSize = 0;
- synchronized (getBlockGCLock()) {
+ synchronized (expiredTombstonesLock) {
// Iterate in reverse order to optimize lots of removes.
// Since expiredTombstones is an ArrayList removing from
// low indexes requires moving everything at a higher index down.
@@ -520,11 +520,13 @@ public class TombstoneService {
// Update the GC RVV for all of the affected regions.
// We need to do this so that we can persist the GC RVV before
// we start removing entries from the map.
- for (Tombstone t : expiredTombstones) {
- DistributedRegion tr = (DistributedRegion) t.region;
- tr.getVersionVector().recordGCVersion(t.getMemberID(), t.getRegionVersion());
- if (!reapedKeys.containsKey(tr)) {
- reapedKeys.put(tr, Collections.emptySet());
+ synchronized (expiredTombstonesLock) {
+ for (Tombstone t : expiredTombstones) {
+ DistributedRegion tr = (DistributedRegion) t.region;
+ tr.getVersionVector().recordGCVersion(t.getMemberID(), t.getRegionVersion());
+ if (!reapedKeys.containsKey(tr)) {
+ reapedKeys.put(tr, Collections.emptySet());
+ }
}
}
@@ -669,7 +671,9 @@ public class TombstoneService {
if (logger.isTraceEnabled(LogMarker.TOMBSTONE)) {
logger.trace(LogMarker.TOMBSTONE, "adding expired tombstone {} to batch", tombstone);
}
- expiredTombstones.add(tombstone);
+ synchronized (expiredTombstonesLock) {
+ expiredTombstones.add(tombstone);
+ }
}
@Override
[2/2] geode git commit: GEODE-2297: push new
InternalDistributedSystem into AlertAppender
Posted by kl...@apache.org.
GEODE-2297: push new InternalDistributedSystem into AlertAppender
* InternalDistributedSystem initialize pushes its instance into
AlertAppender
* During AlertAppender shuttingDown, it nulls out the reference
to InternalDistributedSystem
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/fcf9085f
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/fcf9085f
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/fcf9085f
Branch: refs/heads/feature/GEODE-2297-3
Commit: fcf9085f1e72a72b0507ff56adf360d6d60337e0
Parents: 4d58778
Author: Kirk Lund <kl...@apache.org>
Authored: Fri Jan 20 13:05:22 2017 -0800
Committer: Kirk Lund <kl...@apache.org>
Committed: Mon Jan 23 10:41:30 2017 -0800
----------------------------------------------------------------------
.../internal/InternalDistributedSystem.java | 2 ++
.../internal/logging/log4j/AlertAppender.java | 19 ++++++++++++++++---
2 files changed, 18 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/fcf9085f/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
index fad9206..987e491 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/InternalDistributedSystem.java
@@ -570,6 +570,8 @@ public class InternalDistributedSystem extends DistributedSystem
try {
SocketCreatorFactory.setDistributionConfig(config);
+ AlertAppender.getInstance().onConnect(this);
+
// LOG: create LogWriterAppender(s) if log-file or security-log-file is specified
final boolean hasLogFile =
this.config.getLogFile() != null && !this.config.getLogFile().equals(new File(""));
http://git-wip-us.apache.org/repos/asf/geode/blob/fcf9085f/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java b/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java
index a6c54ab..8a50050 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java
@@ -18,6 +18,7 @@ import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.util.Date;
import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.atomic.AtomicReference;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
@@ -44,7 +45,7 @@ import org.apache.geode.internal.tcp.ReenteredConnectException;
public final class AlertAppender extends AbstractAppender implements PropertyChangeListener {
private static final String APPENDER_NAME = AlertAppender.class.getName();
private static final Logger logger = LogService.getLogger();
- private static final AlertAppender instance = new AlertAppender();
+ private static final AlertAppender instance = createAlertAppender();
/** Is this thread in the process of alerting? */
private static final ThreadLocal<Boolean> alerting = new ThreadLocal<Boolean>() {
@@ -59,12 +60,23 @@ public final class AlertAppender extends AbstractAppender implements PropertyCha
private final AppenderContext appenderContext = LogService.getAppenderContext();
+ private final AtomicReference<InternalDistributedSystem> systemRef = new AtomicReference<>();
+
// This can be set by a loner distributed sytem to disable alerting
private volatile boolean alertingDisabled = false;
+ private static AlertAppender createAlertAppender() {
+ AlertAppender alertAppender = new AlertAppender();
+ alertAppender.start();
+ return alertAppender;
+ }
+
private AlertAppender() {
super(APPENDER_NAME, null, PatternLayout.createDefaultLayout());
- start();
+ }
+
+ public void onConnect(final InternalDistributedSystem system) {
+ this.systemRef.set(system);
}
public static AlertAppender getInstance() {
@@ -114,7 +126,7 @@ public final class AlertAppender extends AbstractAppender implements PropertyCha
logger.debug("Delivering an alert event: {}", event);
}
- InternalDistributedSystem ds = InternalDistributedSystem.getConnectedInstance();
+ InternalDistributedSystem ds = this.systemRef.get();
if (ds == null) {
// Use info level to avoid triggering another alert
logger.info("Did not append alert event because the distributed system is set to null.");
@@ -307,6 +319,7 @@ public final class AlertAppender extends AbstractAppender implements PropertyCha
this.listeners.clear();
this.appenderContext.getLoggerContext().removePropertyChangeListener(this);
this.appenderContext.getLoggerConfig().removeAppender(APPENDER_NAME);
+ this.systemRef.set(null);
}
/**