You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ab...@apache.org on 2017/08/21 19:34:59 UTC
[1/4] geode git commit: GEODE-3314 - Refactoring of DLockService to
improve developer QoL. This now closes #683
Repository: geode
Updated Branches:
refs/heads/release/1.2.1 aa36d3c3c -> 828b24697
GEODE-3314 - Refactoring of DLockService to improve developer QoL. This now closes #683
* Write characterization tests for DLockService.
* Remove debugging code.
* Remove dead code.
* Remove comments.
* Extract the local lock granting into a separate function.
Between the characterization tests we've written and the existing DUnit
tests, the coverage should be fairly adequate.
Signed-off-by: Hitesh Khamesra <hk...@pivotal.io>
Signed-off-by: Galen O'Sullivan <go...@pivotal.io>
(cherry picked from commit 190cfed880da17b947eb520948866062b9aafe0b)
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/26a41458
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/26a41458
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/26a41458
Branch: refs/heads/release/1.2.1
Commit: 26a414582bc718f19a2a94c741132f17bc4ebe53
Parents: be8dde4
Author: Galen O'Sullivan <go...@pivotal.io>
Authored: Wed Aug 2 11:29:21 2017 -0700
Committer: Anthony Baker <ab...@apache.org>
Committed: Mon Aug 21 12:08:57 2017 -0700
----------------------------------------------------------------------
.../internal/locks/DLockRequestProcessor.java | 7 +
.../internal/locks/DLockService.java | 284 +++++--------------
.../distributed/internal/locks/DLockToken.java | 12 +-
.../DLockServiceCharacterizationTests.java | 124 ++++++++
4 files changed, 211 insertions(+), 216 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/26a41458/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
index 3f42adb..96f692b 100755
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
@@ -196,6 +196,13 @@ public class DLockRequestProcessor extends ReplyProcessor21 {
return this.response.leaseExpireTime;
}
+ /**
+ *
+ * @param interruptible
+ * @param lockId
+ * @return
+ * @throws InterruptedException only possible if interruptible is true.
+ */
protected boolean requestLock(boolean interruptible, int lockId) throws InterruptedException {
final boolean isDebugEnabled_DLS = logger.isTraceEnabled(LogMarker.DLS);
http://git-wip-us.apache.org/repos/asf/geode/blob/26a41458/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
index 522b700..f0377b4 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
@@ -17,7 +17,6 @@ package org.apache.geode.distributed.internal.locks;
import org.apache.geode.CancelCriterion;
import org.apache.geode.CancelException;
-import org.apache.geode.InternalGemFireError;
import org.apache.geode.InternalGemFireException;
import org.apache.geode.StatisticsFactory;
import org.apache.geode.SystemFailure;
@@ -74,17 +73,6 @@ public class DLockService extends DistributedLockService {
public static final long NOT_GRANTOR_SLEEP = Long
.getLong(DistributionConfig.GEMFIRE_PREFIX + "DLockService.notGrantorSleep", 100).longValue();
- public static final boolean DEBUG_DISALLOW_NOT_HOLDER = Boolean
- .getBoolean(DistributionConfig.GEMFIRE_PREFIX + "DLockService.debug.disallowNotHolder");
-
- public static final boolean DEBUG_LOCK_REQUEST_LOOP = Boolean
- .getBoolean(DistributionConfig.GEMFIRE_PREFIX + "DLockService.debug.disallowLockRequestLoop");
-
- public static final int DEBUG_LOCK_REQUEST_LOOP_COUNT = Integer
- .getInteger(
- DistributionConfig.GEMFIRE_PREFIX + "DLockService.debug.disallowLockRequestLoopCount", 20)
- .intValue();
-
public static final boolean DEBUG_NONGRANTOR_DESTROY_LOOP = Boolean
.getBoolean(DistributionConfig.GEMFIRE_PREFIX + "DLockService.debug.nonGrantorDestroyLoop");
@@ -93,9 +81,6 @@ public class DLockService extends DistributedLockService {
DistributionConfig.GEMFIRE_PREFIX + "DLockService.debug.nonGrantorDestroyLoopCount", 20)
.intValue();
- public static final boolean DEBUG_ENFORCE_SAFE_EXIT =
- Boolean.getBoolean(DistributionConfig.GEMFIRE_PREFIX + "DLockService.debug.enforceSafeExit");
-
public static final boolean AUTOMATE_FREE_RESOURCES =
Boolean.getBoolean(DistributionConfig.GEMFIRE_PREFIX + "DLockService.automateFreeResources");
@@ -1381,16 +1366,12 @@ public class DLockService extends DistributedLockService {
final boolean disallowReentrant, final boolean disableAlerts) throws InterruptedException {
checkDestroyed();
- final boolean isDebugEnabled_DLS = logger.isTraceEnabled(LogMarker.DLS);
-
boolean interrupted = Thread.interrupted();
if (interrupted && interruptible) {
throw new InterruptedException();
}
- boolean safeExit = true;
- try { // try-block for abnormalExit and safeExit
-
+ try {
long statStart = getStats().startLockWait();
long startTime = getLockTimeStamp(dm);
@@ -1408,9 +1389,7 @@ public class DLockService extends DistributedLockService {
if (waitLimit < 0)
waitLimit = Long.MAX_VALUE;
- if (isDebugEnabled_DLS) {
- logger.trace(LogMarker.DLS, "{}, name: {} - entering lock()", this, name);
- }
+ logger.trace(LogMarker.DLS, "{}, name: {} - entering lock()", this, name);
DLockToken token = getOrCreateToken(name);
boolean gotLock = false;
@@ -1433,29 +1412,7 @@ public class DLockService extends DistributedLockService {
int lockId = -1;
incActiveLocks();
- int loopCount = 0;
while (keepTrying) {
- if (DEBUG_LOCK_REQUEST_LOOP) {
- loopCount++;
- if (loopCount > DEBUG_LOCK_REQUEST_LOOP_COUNT) {
- Integer count = Integer.valueOf(DEBUG_LOCK_REQUEST_LOOP_COUNT);
- String s =
- LocalizedStrings.DLockService_DEBUG_LOCKINTERRUPTIBLY_HAS_GONE_HOT_AND_LOOPED_0_TIMES
- .toLocalizedString(count);
-
- InternalGemFireError e = new InternalGemFireError(s);
- logger.error(LogMarker.DLS,
- LocalizedMessage.create(
- LocalizedStrings.DLockService_DEBUG_LOCKINTERRUPTIBLY_HAS_GONE_HOT_AND_LOOPED_0_TIMES,
- count),
- e);
- throw e;
- }
- /*
- * if (loopCount > 1) { Thread.sleep(1000); }
- */
- }
-
checkDestroyed();
interrupted = Thread.interrupted() || interrupted; // clear
if (interrupted && interruptible) {
@@ -1469,10 +1426,8 @@ public class DLockService extends DistributedLockService {
synchronized (token) {
token.checkForExpiration();
if (token.isLeaseHeldByCurrentThread()) {
- if (isDebugEnabled_DLS) {
- logger.trace(LogMarker.DLS, "{} , name: {} - lock() is reentrant: {}", this, name,
- token);
- }
+ logger.trace(LogMarker.DLS, "{} , name: {} - lock() is reentrant: {}", this, name,
+ token);
reentrant = true;
if (reentrant && disallowReentrant) {
throw new IllegalStateException(
@@ -1480,8 +1435,6 @@ public class DLockService extends DistributedLockService {
.toLocalizedString(new Object[] {Thread.currentThread(), token}));
}
recursionBefore = token.getRecursion();
- leaseExpireTime = token.getLeaseExpireTime(); // moved here from processor null-check
- // under gotLock
lockId = token.getLeaseId(); // keep lockId
if (lockId < 0) {
// loop back around due to expiration
@@ -1500,156 +1453,48 @@ public class DLockService extends DistributedLockService {
lockId = -1; // reset lockId back to -1
}
- DLockRequestProcessor processor = null;
-
- // if reentrant w/ infinite lease TODO: remove false to restore this...
- if (false && reentrant && leaseTimeMillis == Long.MAX_VALUE) {
- // Optimization:
- // thread is reentering lock and lease time is infinite so no
- // need to trouble the poor grantor
- gotLock = true;
- // check for race condition...
- Assert.assertTrue(token.isLeaseHeldByCurrentThread());
- }
-
- // non-reentrant or reentrant w/ non-infinite lease
- else {
- processor = createRequestProcessor(theLockGrantorId, name, threadId, startTime,
- requestLeaseTime, requestWaitTime, reentrant, tryLock, disableAlerts);
- if (reentrant) {
- // check for race condition... reentrant expired already...
- // related to bug 32765, but client-side... see bug 33402
- synchronized (token) {
- if (!token.isLeaseHeldByCurrentThread()) {
- reentrant = false;
- recursionBefore = -1;
- token.checkForExpiration();
- }
+ DLockRequestProcessor processor = createRequestProcessor(theLockGrantorId, name, threadId,
+ startTime, requestLeaseTime, requestWaitTime, reentrant, tryLock, disableAlerts);
+ if (reentrant) {
+ // check for race condition... reentrant expired already...
+ // related to bug 32765, but client-side... see bug 33402
+ synchronized (token) {
+ if (!token.isLeaseHeldByCurrentThread()) {
+ reentrant = false;
+ recursionBefore = -1;
+ token.checkForExpiration();
}
- } else {
- // set lockId since this is the first granting (non-reentrant)
- lockId = processor.getProcessorId();
}
+ } else {
+ // set lockId since this is the first granting (non-reentrant)
+ lockId = processor.getProcessorId();
+ }
- try {
- safeExit = false;
- gotLock = processor.requestLock(interruptible, lockId);
- } catch (InterruptedException e) { // LOST INTERRUPT
- if (interruptible) {
- // TODO: BUG 37158: this can cause a stuck lock
- throw e;
- } else {
- interrupted = true;
- Assert.assertTrue(false,
- "Non-interruptible lock is trying to throw InterruptedException");
- }
- }
- if (isDebugEnabled_DLS) {
- logger.trace(LogMarker.DLS, "Grantor {} replied {}", theLockGrantorId,
- processor.getResponseCodeString());
- }
- } // else: non-reentrant or reentrant w/ non-infinite lease
+ gotLock = processor.requestLock(interruptible, lockId); // can throw
+ // InterruptedException
+
+ logger.trace(LogMarker.DLS, "Grantor {} replied {}", theLockGrantorId,
+ processor.getResponseCodeString());
if (gotLock) {
- // if (processor != null) (cannot be null)
- { // TODO: can be null after restoring above optimization
- // non-reentrant lock needs to getLeaseExpireTime
- leaseExpireTime = processor.getLeaseExpireTime();
- }
+ leaseExpireTime = processor.getLeaseExpireTime();
int recursion = recursionBefore + 1;
- boolean granted = false;
- boolean needToReleaseOrphanedGrant = false;
-
- Assert.assertHoldsLock(this.destroyLock, false);
- synchronized (this.lockGrantorIdLock) {
- if (!checkLockGrantorId(theLockGrantorId)) {
- safeExit = true;
- // race: grantor changed
- if (isDebugEnabled_DLS) {
- logger.trace(LogMarker.DLS,
- "Cannot honor grant from {} because {} is now a grantor.", theLockGrantorId,
- this.lockGrantorId);
- }
- continue;
- } else if (isDestroyed()) {
- // race: dls was destroyed
- if (isDebugEnabled_DLS) {
- logger.trace(LogMarker.DLS,
- "Cannot honor grant from {} because this lock service has been destroyed.",
- theLockGrantorId);
- }
- needToReleaseOrphanedGrant = true;
- } else {
- safeExit = true;
- synchronized (this.tokens) {
- checkDestroyed();
- Assert.assertTrue(token == basicGetToken(name));
- RemoteThread rThread =
- new RemoteThread(getDistributionManager().getId(), threadId);
- granted = token.grantLock(leaseExpireTime, lockId, recursion, rThread);
- } // tokens sync
- }
- }
-
- if (needToReleaseOrphanedGrant /* && processor != null */) {
- processor.getResponse().releaseOrphanedGrant(this.dm);
- safeExit = true;
+ if (!grantLocalDLockAfterObtainingRemoteLock(name, token, threadId, leaseExpireTime,
+ lockId, theLockGrantorId, processor, recursion)) {
continue;
}
- if (!granted) {
- Assert.assertTrue(granted, "Failed to perform client-side granting on " + token
- + " which was granted by " + theLockGrantorId);
- }
-
- // make sure token is THE instance in the map to avoid race with
- // freeResources... ok to overwrite a newer instance too since only
- // one thread will own the lock at a time
- // synchronized (tokens) { // TODO: verify if this is needed
- // synchronized (token) {
- // if (tokens.put(name, token) == null) {
- // getStats().incTokens(1);
- // }
- // }
- // }
-
- if (isDebugEnabled_DLS) {
- logger.trace(LogMarker.DLS, "{}, name: {} - granted lock: {}", this, name, token);
- }
+ logger.trace(LogMarker.DLS, "{}, name: {} - granted lock: {}", this, name, token);
keepTrying = false;
- } // gotLock is true
-
- // grantor replied destroyed (getLock is false)
- else if (processor.repliedDestroyed()) {
- safeExit = true;
- checkDestroyed();
- // should have thrown LockServiceDestroyedException
+ } else if (processor.repliedDestroyed()) {
+ checkDestroyed(); // throws LockServiceDestroyedException
Assert.assertTrue(isDestroyed(),
"Grantor reports service " + this + " is destroyed: " + name);
- } // grantor replied destroyed
-
- // grantor replied NOT_GRANTOR or departed (getLock is false)
- else if (processor.repliedNotGrantor() || processor.hadNoResponse()) {
- safeExit = true;
+ } else if (processor.repliedNotGrantor() || processor.hadNoResponse()) {
notLockGrantorId(theLockGrantorId, 0, TimeUnit.MILLISECONDS);
// keepTrying is still true... loop back around
- } // grantor replied NOT_GRANTOR or departed
-
- // grantor replied NOT_HOLDER for reentrant lock (getLock is false)
- else if (processor.repliedNotHolder()) {
- safeExit = true;
- if (DEBUG_DISALLOW_NOT_HOLDER) {
- String s = LocalizedStrings.DLockService_DEBUG_GRANTOR_REPORTS_NOT_HOLDER_FOR_0
- .toLocalizedString(token);
- InternalGemFireError e = new InternalGemFireError(s);
- logger.error(LogMarker.DLS,
- LocalizedMessage.create(
- LocalizedStrings.DLockService_DEBUG_GRANTOR_REPORTS_NOT_HOLDER_FOR_0, token),
- e);
- throw e;
- }
-
+ } else if (processor.repliedNotHolder()) {
// fix part of bug 32765 - reentrant/expiration problem
// probably expired... try to get non-reentrant lock
reentrant = false;
@@ -1675,7 +1520,6 @@ public class DLockService extends DistributedLockService {
// TODO: figure out when this else case can actually happen...
else {
- safeExit = true;
// either dlock service is suspended or tryLock failed
// fixed the math here... bug 32765
if (waitLimit > token.getCurrentTime() + 20) {
@@ -1685,10 +1529,8 @@ public class DLockService extends DistributedLockService {
}
} // while (keepTrying)
- } // try-block for end stats, token cleanup, and interrupt check
-
- // finally-block for end stats, token cleanup, and interrupt check
- finally {
+ // try-block for end stats, token cleanup, and interrupt check
+ } finally {
getStats().endLockWait(statStart, gotLock);
// cleanup token if failed to get lock
@@ -1711,26 +1553,50 @@ public class DLockService extends DistributedLockService {
blockedOn.set(null);
}
- if (isDebugEnabled_DLS) {
- logger.trace(LogMarker.DLS, "{}, name: {} - exiting lock() returning {}", this, name,
- gotLock);
- }
+ logger.trace(LogMarker.DLS, "{}, name: {} - exiting lock() returning {}", this, name,
+ gotLock);
return gotLock;
- } // try-block for abnormalExit and safeExit
-
- // finally-block for abnormalExit and safeExit
- finally {
- if (isDebugEnabled_DLS) {
- logger.trace(LogMarker.DLS, "{}, name: {} - exiting lock() without returning value", this,
- name);
- }
+ } finally {
+ logger.trace(LogMarker.DLS, "{}, name: {} - exiting lock() without returning value", this,
+ name);
if (interrupted) {
Thread.currentThread().interrupt();
}
- if (DEBUG_ENFORCE_SAFE_EXIT) {
- Assert.assertTrue(safeExit);
+ }
+ }
+
+ private boolean grantLocalDLockAfterObtainingRemoteLock(Object name, DLockToken token,
+ int threadId, long leaseExpireTime, int lockId, LockGrantorId theLockGrantorId,
+ DLockRequestProcessor processor, int recursion) {
+ boolean needToReleaseOrphanedGrant = false;
+
+ Assert.assertHoldsLock(this.destroyLock, false);
+ synchronized (this.lockGrantorIdLock) {
+ if (!checkLockGrantorId(theLockGrantorId)) {
+ // race: grantor changed
+ logger.trace(LogMarker.DLS, "Cannot honor grant from {} because {} is now a grantor.",
+ theLockGrantorId, this.lockGrantorId);
+ } else if (isDestroyed()) {
+ // race: dls was destroyed
+ logger.trace(LogMarker.DLS,
+ "Cannot honor grant from {} because this lock service has been destroyed.",
+ theLockGrantorId);
+ needToReleaseOrphanedGrant = true;
+ } else {
+ synchronized (this.tokens) {
+ checkDestroyed();
+ Assert.assertTrue(token == basicGetToken(name));
+ RemoteThread rThread = new RemoteThread(getDistributionManager().getId(), threadId);
+ token.grantLock(leaseExpireTime, lockId, recursion, rThread);
+ return true;
+ } // tokens sync
}
}
+
+ if (needToReleaseOrphanedGrant) {
+ processor.getResponse().releaseOrphanedGrant(this.dm);
+ }
+ return false;
}
/**
@@ -2547,11 +2413,11 @@ public class DLockService extends DistributedLockService {
/**
* Called by grantor recovery to return set of locks held by this process. Synchronizes on
* lockGrantorIdLock, tokens map, and each lock token.
- *
+ *
* @param newlockGrantorId the newly recovering grantor
*/
- Set getLockTokensForRecovery(LockGrantorId newlockGrantorId) {
- Set heldLockSet = Collections.EMPTY_SET;
+ Set<DLockRemoteToken> getLockTokensForRecovery(LockGrantorId newlockGrantorId) {
+ Set<DLockRemoteToken> heldLockSet = Collections.EMPTY_SET;
LockGrantorId currentLockGrantorId = null;
synchronized (this.lockGrantorIdLock) {
@@ -2589,7 +2455,7 @@ public class DLockService extends DistributedLockService {
// add token to heldLockSet
else {
if (heldLockSet == Collections.EMPTY_SET) {
- heldLockSet = new HashSet();
+ heldLockSet = new HashSet<>();
}
heldLockSet.add(DLockRemoteToken.createFromDLockToken(token));
}
http://git-wip-us.apache.org/repos/asf/geode/blob/26a41458/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockToken.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockToken.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockToken.java
index c67de67..3e85171 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockToken.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockToken.java
@@ -87,7 +87,8 @@ public class DLockToken {
private Thread thread;
/**
- * Number of threads currently using this lock token.
+ * Number of usages of this lock token. usageCount = recursion + (# of threads waiting for this
+ * lock). It's weird, I know.
*/
private int usageCount = 0;
@@ -230,10 +231,9 @@ public class DLockToken {
// -------------------------------------------------------------------------
/**
- * Destroys this lock token. Caller must synchronize on this lock token.
+ * Destroys this lock token.
*/
synchronized void destroy() {
- // checkDestroyed();
this.destroyed = true;
}
@@ -302,14 +302,14 @@ public class DLockToken {
* @param remoteThread identity of the leasing thread
* @return true if lease for this lock token is successfully granted
*/
- synchronized boolean grantLock(long newLeaseExpireTime, int newLeaseId, int newRecursion,
+ synchronized void grantLock(long newLeaseExpireTime, int newLeaseId, int newRecursion,
RemoteThread remoteThread) {
Assert.assertTrue(remoteThread != null);
Assert.assertTrue(newLeaseId > -1, "Invalid attempt to grant lock with leaseId " + newLeaseId);
checkDestroyed();
- checkForExpiration();
+ checkForExpiration(); // TODO: this should throw.
this.ignoreForRecovery = false;
this.leaseExpireTime = newLeaseExpireTime;
@@ -321,8 +321,6 @@ public class DLockToken {
if (logger.isTraceEnabled(LogMarker.DLS)) {
logger.trace(LogMarker.DLS, "[DLockToken.grantLock.client] granted {}", this);
}
-
- return true;
}
/**
http://git-wip-us.apache.org/repos/asf/geode/blob/26a41458/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceCharacterizationTests.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceCharacterizationTests.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceCharacterizationTests.java
new file mode 100644
index 0000000..ba300c4
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceCharacterizationTests.java
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.distributed.internal.locks;
+
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.awaitility.Awaitility.await;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.ExpirationAction;
+import org.apache.geode.cache.ExpirationAttributes;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.distributed.DistributedLockService;
+import org.apache.geode.internal.cache.DistributedRegion;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+public class DLockServiceCharacterizationTests {
+ private Cache cache;
+ private DistributedRegion testRegion;
+ private DistributedLockService dLockService;
+
+ @Before
+ public void setUp() {
+ Properties properties = new Properties();
+ properties.setProperty(MCAST_PORT, "0");
+
+ cache = new CacheFactory(properties).create();
+ testRegion = (DistributedRegion) cache.createRegionFactory(RegionShortcut.REPLICATE)
+ .setScope(Scope.GLOBAL)
+ .setEntryTimeToLive(new ExpirationAttributes(1, ExpirationAction.DESTROY))
+ .create("testRegion");
+ testRegion.becomeLockGrantor();
+
+ dLockService = DLockService.create("testService", cache.getDistributedSystem());
+ }
+
+ @After
+ public void tearDown() {
+ cache.close();
+ }
+
+ @Test
+ public void reentrantLockIncreasesReentrancy() {
+ assertTrue(dLockService.lock("key1", -1, -1));
+ DLockToken key1 = ((DLockService) dLockService).getToken("key1");
+
+ assertEquals(0, key1.getRecursion());
+ assertEquals(1, key1.getUsageCount());
+ // reentrancy + 1
+ assertTrue(dLockService.lock("key1", -1, -1));
+
+ assertEquals(1, key1.getRecursion());
+ assertEquals(2, key1.getUsageCount());
+
+ dLockService.unlock("key1");
+ assertEquals(0, key1.getRecursion());
+ assertEquals(1, key1.getUsageCount());
+
+ dLockService.unlock("key1");
+ assertTokenIsUnused(key1);
+ }
+
+ @Test
+ public void threadWaitingOnLockIncreasesUsageCount() {
+ assertTrue(dLockService.lock("key1", -1, -1));
+ DLockToken key1 = ((DLockService) dLockService).getToken("key1");
+
+ assertEquals(0, key1.getRecursion());
+ assertEquals(1, key1.getUsageCount());
+ assertEquals(Thread.currentThread(), key1.getThread());
+
+ Thread otherThread = new Thread(() -> dLockService.lock("key1", -1, -1));
+ otherThread.start();
+
+ // otherThread should be waiting for lock.
+
+ await("other thread is waiting on this lock").atMost(3, TimeUnit.SECONDS)
+ .until(() -> key1.getUsageCount() == 2);
+ assertEquals(0, key1.getRecursion());
+ assertEquals(Thread.currentThread(), key1.getThread());
+
+ dLockService.unlock("key1");
+
+ await("other thread has acquired this lock").atMost(3, TimeUnit.SECONDS)
+ .until(() -> key1.getThread() == otherThread);
+
+ assertEquals(0, key1.getRecursion());
+ assertEquals(1, key1.getUsageCount());
+
+ // We can unlock from a different thread than locked it.
+ dLockService.unlock("key1");
+
+ assertTokenIsUnused(key1);
+ }
+
+ private void assertTokenIsUnused(DLockToken dLockToken) {
+ assertEquals(0, dLockToken.getRecursion());
+ assertEquals(0, dLockToken.getUsageCount());
+ assertEquals(null, dLockToken.getThread());
+ assertEquals(null, dLockToken.getLesseeThread());
+ assertEquals(-1, dLockToken.getLeaseId());
+ }
+}
[2/4] geode git commit: GEODE-3314: Fix DLockService token leak.
Posted by ab...@apache.org.
GEODE-3314: Fix DLockService token leak.
And add a test so that we don't do that again.
We had been unlocking the DLock on the remote server before freeing the
local `DLockToken`. This meant that the local token got reused by
another thread acquiring the same lock and the count went up and up and
up (due to some logic that wouldn't decrease the count if it no longer
owned the token).
Tokens should no longer have usageCounts that climb over time, which
means they should be released normally by freeResources.
Signed-off-by: Bruce Schuchardt <bs...@pivotal.io>
Signed-off-by: Galen O'Sullivan <go...@pivotal.io>
This closes #663
Signed-off-by: Hitesh Khamesra <hk...@pivotal.io>
(cherry picked from commit d91096c19a453e3d04f9b3d9151ad57dc8e79e0a)
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/be8dde4f
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/be8dde4f
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/be8dde4f
Branch: refs/heads/release/1.2.1
Commit: be8dde4fe287bacd632d7172d0efb70dbaf7455f
Parents: 49220c3
Author: Galen O'Sullivan <go...@pivotal.io>
Authored: Thu Jul 27 16:35:04 2017 -0700
Committer: Anthony Baker <ab...@apache.org>
Committed: Mon Aug 21 12:08:57 2017 -0700
----------------------------------------------------------------------
.../internal/locks/DLockRequestProcessor.java | 6 -
.../internal/locks/DLockService.java | 10 +-
.../internal/locks/DLockServiceLeakTest.java | 158 +++++++++++++++++++
3 files changed, 162 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/be8dde4f/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
index f0ee31b..3f42adb 100755
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
@@ -202,18 +202,12 @@ public class DLockRequestProcessor extends ReplyProcessor21 {
Assert.assertTrue(lockId > -1, "lockId is < 0: " + this);
this.request.lockId = lockId;
- // setDoneProcessing(false);
-
// local grantor... don't use messaging... fake it
if (isLockGrantor()) {
if (isDebugEnabled_DLS) {
logger.trace(LogMarker.DLS, "DLockRequestProcessor processing lock request directly");
}
this.request.setSender(this.dm.getDistributionManagerId());
- /*
- * if (svc.isDestroyed()) { return false; }
- */
- // svc.checkDestroyed();
// calls processor (this) process...
this.request.processLocally(this.dm);
http://git-wip-us.apache.org/repos/asf/geode/blob/be8dde4f/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
index c38cdad..522b700 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
@@ -1388,7 +1388,6 @@ public class DLockService extends DistributedLockService {
throw new InterruptedException();
}
- boolean abnormalExit = true;
boolean safeExit = true;
try { // try-block for abnormalExit and safeExit
@@ -1716,7 +1715,6 @@ public class DLockService extends DistributedLockService {
logger.trace(LogMarker.DLS, "{}, name: {} - exiting lock() returning {}", this, name,
gotLock);
}
- abnormalExit = false;
return gotLock;
} // try-block for abnormalExit and safeExit
@@ -1929,12 +1927,12 @@ public class DLockService extends DistributedLockService {
checkDestroyed();
LockGrantorId theLockGrantorId = getLockGrantorId();
try {
- released = callReleaseProcessor(theLockGrantorId.getLockGrantorMember(), name,
- lockBatch, lockId);
synchronized (this.lockGrantorIdLock) {
- token.releaseLock(lockId, rThread);
- unlocked = true;
+ unlocked = token.releaseLock(lockId, rThread);
}
+ released = callReleaseProcessor(theLockGrantorId.getLockGrantorMember(), name,
+ lockBatch, lockId);
+
} catch (LockGrantorDestroyedException e) { // part of fix for bug 35239
// loop back around to get next lock grantor
} catch (LockServiceDestroyedException e) { // part of fix for bug 35239
http://git-wip-us.apache.org/repos/asf/geode/blob/be8dde4f/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceLeakTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceLeakTest.java b/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceLeakTest.java
new file mode 100644
index 0000000..bd85eb9
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceLeakTest.java
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.distributed.internal.locks;
+
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.ExpirationAction;
+import org.apache.geode.cache.ExpirationAttributes;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.internal.cache.DistributedRegion;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.Collection;
+import java.util.LinkedList;
+import java.util.Properties;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.locks.Lock;
+
+@Category(IntegrationTest.class)
+public class DLockServiceLeakTest {
+ private Cache cache;
+ private ExecutorService executorService;
+ private DistributedRegion testRegion;
+
+ @Before
+ public void setUp() {
+ Properties properties = new Properties();
+ properties.setProperty(MCAST_PORT, "0");
+
+ cache = new CacheFactory(properties).create();
+ testRegion = (DistributedRegion) cache.createRegionFactory(RegionShortcut.REPLICATE)
+ .setScope(Scope.GLOBAL)
+ .setEntryTimeToLive(new ExpirationAttributes(1, ExpirationAction.DESTROY))
+ .create("testRegion");
+ testRegion.becomeLockGrantor();
+
+ executorService = Executors.newFixedThreadPool(5);
+ }
+
+ @After
+ public void tearDown() {
+ cache.close();
+ executorService.shutdownNow();
+ }
+
+ @Test
+ public void basicDLockUsage() throws InterruptedException, ExecutionException, TimeoutException {
+ Lock lock = testRegion.getDistributedLock("testLockName");
+ lock.lockInterruptibly();
+
+ Future<Boolean> future = executorService.submit(() -> lock.tryLock());
+ assertFalse("should not be able to get lock from another thread",
+ future.get(5, TimeUnit.SECONDS));
+
+ assertTrue("Lock is reentrant", lock.tryLock());
+ // now locked twice.
+
+ future = executorService.submit(() -> lock.tryLock());
+ assertFalse("should not be able to get lock from another thread",
+ future.get(5, TimeUnit.SECONDS));
+
+ lock.unlock();
+
+ future = executorService.submit(() -> lock.tryLock());
+ assertFalse("should not be able to get lock from another thread", future.get());
+
+ lock.unlock();
+
+ future = executorService.submit(() -> {
+ boolean locked = lock.tryLock();
+ if (!locked) {
+ return false;
+ }
+ lock.unlock();
+ return true;
+ });
+ assertTrue("Another thread can now take out the lock", future.get(5, TimeUnit.SECONDS));
+
+ DLockService lockService = (DLockService) testRegion.getLockService();
+ Collection<DLockToken> tokens = lockService.getTokens();
+
+ for (DLockToken token : tokens) {
+ assertEquals(0, token.getUsageCount());
+ }
+ }
+
+ @Test
+ public void singleThreadWithCache() {
+ putTestKey();
+
+ DLockService lockService = (DLockService) testRegion.getLockService();
+ Collection<DLockToken> tokens = lockService.getTokens();
+
+ assertEquals(1, tokens.size());
+
+ for (DLockToken token : tokens) {
+ assertEquals(0, token.getUsageCount());
+ }
+ }
+
+ @Test
+ public void multipleThreadsWithCache() {
+ LinkedList<Future> futures = new LinkedList<>();
+ for (int i = 0; i < 5; i++) {
+ futures.add(executorService.submit(this::putTestKey));
+ }
+
+ futures.forEach(future -> {
+ try {
+ future.get();
+ } catch (InterruptedException | ExecutionException e) {
+ e.printStackTrace();
+ }
+ });
+
+ DLockService lockService = (DLockService) testRegion.getLockService();
+ Collection<DLockToken> tokens = lockService.getTokens();
+
+ assertTrue(tokens.size() < 2);
+
+ for (DLockToken token : tokens) {
+ assertEquals(0, token.getUsageCount());
+ }
+ }
+
+ private void putTestKey() {
+ for (int i = 0; i < 1000; i++) {
+ testRegion.put("testKey", "testValue");
+ }
+ }
+}
[3/4] geode git commit: GEODE-3393: One-way SSL commit failing with
userHome/.keystore not found. This now closes #682
Posted by ab...@apache.org.
GEODE-3393: One-way SSL commit failing with userHome/.keystore not found. This now closes #682
Signed-off-by: Galen O'Sullivan <go...@pivotal.io>
(cherry picked from commit 684f85d2881dd1b0b68bc49b303fb45a8b17452d)
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/49220c3b
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/49220c3b
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/49220c3b
Branch: refs/heads/release/1.2.1
Commit: 49220c3be5326c6a58c36f0fca8fedc00ba5c7d1
Parents: aa36d3c
Author: Udo Kohlmeyer <uk...@pivotal.io>
Authored: Thu Aug 3 14:13:06 2017 -0700
Committer: Anthony Baker <ab...@apache.org>
Committed: Mon Aug 21 12:08:57 2017 -0700
----------------------------------------------------------------------
.../apache/geode/internal/admin/SSLConfig.java | 5 ++-
.../geode/internal/net/SocketCreator.java | 38 ++++++-----------
.../net/SSLConfigurationFactoryJUnitTest.java | 6 ++-
.../internal/net/SocketCreatorJUnitTest.java | 43 ++++++++++++++++++++
4 files changed, 62 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/49220c3b/geode-core/src/main/java/org/apache/geode/internal/admin/SSLConfig.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/admin/SSLConfig.java b/geode-core/src/main/java/org/apache/geode/internal/admin/SSLConfig.java
index 0171933..65e4694 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/admin/SSLConfig.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/admin/SSLConfig.java
@@ -16,6 +16,7 @@ package org.apache.geode.internal.admin;
import static org.apache.geode.distributed.ConfigurationProperties.*;
+import java.security.KeyStore;
import java.util.Iterator;
import java.util.Properties;
@@ -33,11 +34,11 @@ public class SSLConfig {
private String ciphers = DistributionConfig.DEFAULT_SSL_CIPHERS;
private boolean requireAuth = DistributionConfig.DEFAULT_SSL_REQUIRE_AUTHENTICATION;
private String keystore = DistributionConfig.DEFAULT_SSL_KEYSTORE;
- private String keystoreType = DistributionConfig.DEFAULT_CLUSTER_SSL_KEYSTORE_TYPE;
+ private String keystoreType = KeyStore.getDefaultType();
private String keystorePassword = DistributionConfig.DEFAULT_SSL_KEYSTORE_PASSWORD;
private String truststore = DistributionConfig.DEFAULT_SSL_TRUSTSTORE;
private String truststorePassword = DistributionConfig.DEFAULT_SSL_TRUSTSTORE_PASSWORD;
- private String truststoreType = DistributionConfig.DEFAULT_CLUSTER_SSL_KEYSTORE_TYPE;
+ private String truststoreType = KeyStore.getDefaultType();
private String alias = null;
private SecurableCommunicationChannel securableCommunicationChannel = null;
http://git-wip-us.apache.org/repos/asf/geode/blob/49220c3b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
index fec81ca..5f4cfb1 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
@@ -331,7 +331,6 @@ public class SocketCreator {
* <p>
* Caller must synchronize on the SocketCreator instance.
*/
- @SuppressWarnings("hiding")
private void initialize() {
try {
// set p2p values...
@@ -382,7 +381,7 @@ public class SocketCreator {
/**
* Creates & configures the SSLContext when SSL is enabled.
- *
+ *
* @return new SSLContext configured using the given protocols & properties
*
* @throws GeneralSecurityException if security information can not be found
@@ -400,7 +399,7 @@ public class SocketCreator {
/**
* Used by CacheServerLauncher and SystemAdmin to read the properties from console
- *
+ *
* @param env Map in which the properties are to be read from console.
*/
public static void readSSLProperties(Map<String, String> env) {
@@ -411,7 +410,7 @@ public class SocketCreator {
* Used to read the properties from console. AgentLauncher calls this method directly & ignores
* gemfire.properties. CacheServerLauncher and SystemAdmin call this through
* {@link #readSSLProperties(Map)} and do NOT ignore gemfire.properties.
- *
+ *
* @param env Map in which the properties are to be read from console.
* @param ignoreGemFirePropsFile if <code>false</code> existing gemfire.properties file is read,
* if <code>true</code>, properties from gemfire.properties file are ignored.
@@ -541,6 +540,10 @@ public class SocketCreator {
NoSuchAlgorithmException, CertificateException, UnrecoverableKeyException {
GfeConsoleReader consoleReader = GfeConsoleReaderFactory.getDefaultConsoleReader();
+ if (sslConfig.getKeystore() == null) {
+ return null;
+ }
+
KeyManager[] keyManagers = null;
String keyStoreType = sslConfig.getKeystoreType();
if (StringUtils.isEmpty(keyStoreType)) {
@@ -615,7 +618,7 @@ public class SocketCreator {
/**
* Constructor.
- *
+ *
* @param mgr The X509KeyManager used as a delegate
* @param keyAlias The alias name of the server's keypair and supporting certificate chain
*/
@@ -795,7 +798,7 @@ public class SocketCreator {
/**
* Creates or bind server socket to a random port selected from tcp-port-range which is same as
* membership-port-range.
- *
+ *
* @param ba
* @param backlog
* @param isBindAddress
@@ -815,7 +818,7 @@ public class SocketCreator {
/**
* Creates or bind server socket to a random port selected from tcp-port-range which is same as
* membership-port-range.
- *
+ *
* @param ba
* @param backlog
* @param isBindAddress
@@ -1025,14 +1028,6 @@ public class SocketCreator {
ex);
throw ex;
}
- } catch (SSLException ex) {
- logger
- .fatal(
- LocalizedMessage.create(
- LocalizedStrings.SocketCreator_SSL_ERROR_IN_CONNECTING_TO_PEER_0_1,
- new Object[] {socket.getInetAddress(), Integer.valueOf(socket.getPort())}),
- ex);
- throw ex;
}
}
}
@@ -1112,16 +1107,7 @@ public class SocketCreator {
.create(LocalizedStrings.SocketCreator_SSL_ERROR_IN_AUTHENTICATING_PEER), ex);
throw ex;
}
- } catch (SSLException ex) {
- logger
- .fatal(
- LocalizedMessage.create(
- LocalizedStrings.SocketCreator_SSL_ERROR_IN_CONNECTING_TO_PEER_0_1,
- new Object[] {socket.getInetAddress(), Integer.valueOf(socket.getPort())}),
- ex);
- throw ex;
}
-
}
}
@@ -1223,7 +1209,7 @@ public class SocketCreator {
/**
* This method uses JNDI to look up an address in DNS and return its name
- *
+ *
* @param addr
*
* @return the host name associated with the address or null if lookup isn't possible or there is
@@ -1299,7 +1285,7 @@ public class SocketCreator {
* Fails Assertion if the conversion would result in <code>java.lang.UnknownHostException</code>.
* <p>
* Any leading slashes on host will be ignored.
- *
+ *
* @param host string version the InetAddress
*
* @return the host converted to InetAddress instance
http://git-wip-us.apache.org/repos/asf/geode/blob/49220c3b/geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
index 31c2469..1d2fe70 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/net/SSLConfigurationFactoryJUnitTest.java
@@ -49,6 +49,8 @@ import org.junit.experimental.categories.Category;
import java.util.Properties;
+import java.security.KeyStore;
+
@Category({UnitTest.class, MembershipTest.class})
public class SSLConfigurationFactoryJUnitTest {
@@ -214,11 +216,11 @@ public class SSLConfigurationFactoryJUnitTest {
properties.setProperty(CLUSTER_SSL_ENABLED, "true");
properties.setProperty(MCAST_PORT, "0");
System.setProperty(SSLConfigurationFactory.JAVAX_KEYSTORE, "keystore");
- System.setProperty(SSLConfigurationFactory.JAVAX_KEYSTORE_TYPE, "JKS");
+ System.setProperty(SSLConfigurationFactory.JAVAX_KEYSTORE_TYPE, KeyStore.getDefaultType());
System.setProperty(SSLConfigurationFactory.JAVAX_KEYSTORE_PASSWORD, "keystorePassword");
System.setProperty(SSLConfigurationFactory.JAVAX_TRUSTSTORE, "truststore");
System.setProperty(SSLConfigurationFactory.JAVAX_TRUSTSTORE_PASSWORD, "truststorePassword");
- System.setProperty(SSLConfigurationFactory.JAVAX_TRUSTSTORE_TYPE, "JKS");
+ System.setProperty(SSLConfigurationFactory.JAVAX_TRUSTSTORE_TYPE, KeyStore.getDefaultType());
DistributionConfigImpl distributionConfig = new DistributionConfigImpl(properties);
SSLConfigurationFactory.setDistributionConfig(distributionConfig);
SSLConfig sslConfig =
http://git-wip-us.apache.org/repos/asf/geode/blob/49220c3b/geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorJUnitTest.java
new file mode 100644
index 0000000..b258ee1
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorJUnitTest.java
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.internal.net;
+
+import org.apache.geode.internal.admin.SSLConfig;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.apache.geode.util.test.TestUtil;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(UnitTest.class)
+public class SocketCreatorJUnitTest {
+
+ @Test
+ public void testCreateSocketCreatorWithKeystoreUnset() throws Exception {
+ SSLConfig testSSLConfig = new SSLConfig();
+ testSSLConfig.setEnabled(true);
+ testSSLConfig.setKeystore(null);
+ testSSLConfig.setKeystorePassword("");
+ testSSLConfig.setTruststore(getSingleKeyKeystore());
+ testSSLConfig.setTruststorePassword("password");
+ // GEODE-3393: This would fail with java.io.FileNotFoundException: $USER_HOME/.keystore
+ new SocketCreator(testSSLConfig);
+
+ }
+
+ private String getSingleKeyKeystore() {
+ return TestUtil.getResourcePath(getClass(), "/ssl/trusted.keystore");
+ }
+
+}
[4/4] geode git commit: GEODE-3410 Doc update for gfsh query command
changes
Posted by ab...@apache.org.
GEODE-3410 Doc update for gfsh query command changes
- the --interactive option is gone
- there is a new --file option
- correct default value for APP_FETCH_SIZE gfsh env var
- gfsh env var APP_COLLECTION_LIMIT is no longer used
This closes #708
(cherry picked from commit 7528f1290aa8fc343787432d163cf43dd3fdfe21)
Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/828b2469
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/828b2469
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/828b2469
Branch: refs/heads/release/1.2.1
Commit: 828b24697823ee665b301ce3c54dbb15d0ccbce5
Parents: 26a4145
Author: Karen Miller <km...@pivotal.io>
Authored: Thu Aug 10 12:53:25 2017 -0700
Committer: Anthony Baker <ab...@apache.org>
Committed: Mon Aug 21 12:12:56 2017 -0700
----------------------------------------------------------------------
.../gfsh/command-pages/query.html.md.erb | 24 ++++++++++----------
.../gfsh/configuring_gfsh.html.md.erb | 5 ++--
.../useful_gfsh_shell_variables.html.md.erb | 19 ++++++++--------
3 files changed, 24 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/geode/blob/828b2469/geode-docs/tools_modules/gfsh/command-pages/query.html.md.erb
----------------------------------------------------------------------
diff --git a/geode-docs/tools_modules/gfsh/command-pages/query.html.md.erb b/geode-docs/tools_modules/gfsh/command-pages/query.html.md.erb
index 1ff3028..8df594e 100644
--- a/geode-docs/tools_modules/gfsh/command-pages/query.html.md.erb
+++ b/geode-docs/tools_modules/gfsh/command-pages/query.html.md.erb
@@ -21,7 +21,13 @@ limitations under the License.
Run queries against Geode regions.
-Run the specified OQL query as a single quoted string and displays results in pages allows to move between pages. If limit is not set in the query, then a default limit of 1000 (derived from GFSH environment variable APP\_FETCH\_SIZE) will be applied. Page size is derived from GFSH environment variable APP\_COLLECTION\_LIMIT (default value=20).
+If a limit restricting the result size is not set in the query,
+then a default limit of the gfsh environment variable `APP_FETCH_SIZE`,
+as defined in
+[Useful gfsh Shell Variables](../useful_gfsh_shell_variables.html),
+will be applied.
+
+Surround the OQL query with single quote marks.
**Note:**
This command should not be executed from `gfsh` if the objects being queried contain cyclic references.
@@ -31,24 +37,18 @@ This command should not be executed from `gfsh` if the objects being queried con
**Syntax:**
``` pre
-query --query=value [--step-name=value] [--interactive=value]
+query --query=value [--step-name=value] [--file=path/to/results/file]
```
<a id="concept_89A129F729DF4D3B9056C8D9016AA760__table_ocr_gcg_2w"></a>
-| Name | Description | Default Value |
-|-----------------------------------------------------|-----------------------------------------------------------------------------------------------------------|---------------|
-| <span class="keyword parmname">\\-\\-query </span> | *Required.* The OQL string. | |
-| <span class="keyword parmname">\\-\\-interactive</span> | Specifies whether or not this query is interactive. If false, then all results will be displayed at once. | true |
+| Name | Description |
+|-----------------------------------------------------|-----------------------------------------------------------------------------------------------------------|
+| <span class="keyword parmname">\\-\\-query </span> | *Required.* The OQL string. |
+| <span class="keyword parmname">\\-\\-file</span> | When specified, all query results are written to the specified file. An error is issued if the file already exists. |
<span class="tablecap">Table 1. Query Parameters</span>
-**Example Commands:**
-
-``` pre
-query --query=value [--step-name=value] [--interactive=value]
-```
-
**Sample Output:**
``` pre
http://git-wip-us.apache.org/repos/asf/geode/blob/828b2469/geode-docs/tools_modules/gfsh/configuring_gfsh.html.md.erb
----------------------------------------------------------------------
diff --git a/geode-docs/tools_modules/gfsh/configuring_gfsh.html.md.erb b/geode-docs/tools_modules/gfsh/configuring_gfsh.html.md.erb
index b112e66..d4cdea0 100644
--- a/geode-docs/tools_modules/gfsh/configuring_gfsh.html.md.erb
+++ b/geode-docs/tools_modules/gfsh/configuring_gfsh.html.md.erb
@@ -48,10 +48,11 @@ You may also need to verify that the ports are available and open to client conn
## <a id="concept_3B9C6CE2F64841E98C33D9F6441DF487__section_C69A2711A7664A9091A5E634221053CB" class="no-quick-link"></a>Configuring gfsh Environment Variables
-In addition, you can set several gfsh-specific preset SHELL variables by using the `set variable` command. For example, you can set `gfsh` to run in quiet mode. Not all `gfsh` variables are modifiable; user-configurable variables include:
+In addition, you can set gfsh-specific preset SHELL variables by using the `set variable` command. For example, you can set `gfsh` to run in quiet mode.
+Not all `gfsh` variables are modifiable.
+User-configurable variables include:
- APP\_FETCH\_SIZE
-- APP\_COLLECTION\_LIMIT
- APP\_QUIET\_EXECUTION
See [Useful gfsh Shell Variables](useful_gfsh_shell_variables.html#concept_731ECA5E40E943CBA5C1198A0745D8EE) for more information.
http://git-wip-us.apache.org/repos/asf/geode/blob/828b2469/geode-docs/tools_modules/gfsh/useful_gfsh_shell_variables.html.md.erb
----------------------------------------------------------------------
diff --git a/geode-docs/tools_modules/gfsh/useful_gfsh_shell_variables.html.md.erb b/geode-docs/tools_modules/gfsh/useful_gfsh_shell_variables.html.md.erb
index 731953c..e355e9f 100644
--- a/geode-docs/tools_modules/gfsh/useful_gfsh_shell_variables.html.md.erb
+++ b/geode-docs/tools_modules/gfsh/useful_gfsh_shell_variables.html.md.erb
@@ -58,15 +58,14 @@ gfsh>echo --string=${SYS_CLASSPATH}
| | |
|------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| APP\_COLLECTION\_LIMIT | Number of items in the embedded collection of a result to be iterated Values: 1-100. Default value is 20. |
-| APP\_FETCH\_SIZE | Fetch size to be used while querying. Values: 0 - 2147483647. Default value is 1000. |
-| APP\_LAST\_EXIT\_STATUS | Last command exit status. Similar to $? (Unix) and %errorlevel% (Windows). Values: 0 (successful), 1 (error), 2(crash) (read only). |
-| APP\_LOGGING\_ENABLED | Whether gfsh logging is enabled. Default: false (read only). You can enable gfsh logging by setting the `gfsh.log-level` Java system property to a [supported Java log level](http://docs.oracle.com/javase/8/docs/api/java/util/logging/Level.html). |
-| APP\_LOG\_FILE | Path and name of current gfsh log file (read only). |
-| APP\_NAME | Name of the application-- "gfsh" (read only). |
-| APP\_PWD | Current working directory where gfsh was launched (read only). |
-| APP\_QUERY\_RESULTS\_DISPLAY\_MODE | Toggle the display mode for returning query results. Values: table or catalog. Default value is table. |
-| APP\_QUIET\_EXECUTION | Whether the execution should be in quiet mode. Values (case insensitive): true, false. Default value is false. |
-| APP\_RESULT\_VIEWER | Unix only. Set this variable to `external` to enable viewing of the output using the UNIX `less` command. Default value is basic (gfsh). |
+| APP\_FETCH\_SIZE | Fetch size to be used while querying. Values: 0 - 2147483647. Default value is 100. |
+| APP\_LAST\_EXIT\_STATUS | Last command exit status. Similar to $? (Unix) and %errorlevel% (Windows). Values: 0 (successful), 1 (error), 2(crash) (read only). |
+| APP\_LOGGING\_ENABLED | Whether gfsh logging is enabled. Default: false (read only). You can enable gfsh logging by setting the `gfsh.log-level` Java system property to a [supported Java log level](http://docs.oracle.com/javase/8/docs/api/java/util/logging/Level.html).|
+| APP\_LOG\_FILE | Path and name of current gfsh log file (read only). |
+| APP\_NAME | Name of the application-- "gfsh" (read only). |
+| APP\_PWD | Current working directory where gfsh was launched (read only). |
+| APP\_QUERY\_RESULTS\_DISPLAY\_MODE | Toggle the display mode for returning query results. Values: table or catalog. Default value is table. |
+| APP\_QUIET\_EXECUTION | Whether the execution should be in quiet mode. Values (case insensitive): true, false. Default value is false. |
+| APP\_RESULT\_VIEWER | Unix only. Set this variable to `external` to enable viewing of the output using the UNIX `less` command. Default value is basic (gfsh). |