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/04/27 20:53:38 UTC

geode git commit: GEODE-2632: minor fixes from code review

Repository: geode
Updated Branches:
  refs/heads/develop a48be603d -> f1b14b0dd


GEODE-2632: minor fixes from code review

* add TODO comments for some larger fixes from review


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/f1b14b0d
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/f1b14b0d
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/f1b14b0d

Branch: refs/heads/develop
Commit: f1b14b0dda058ccdd6f82075edeb3fa62245c6ae
Parents: a48be60
Author: Kirk Lund <kl...@apache.org>
Authored: Wed Apr 26 10:06:01 2017 -0700
Committer: Kirk Lund <kl...@apache.org>
Committed: Thu Apr 27 13:53:32 2017 -0700

----------------------------------------------------------------------
 .../internal/DistributionManager.java           |  6 ++++-
 .../geode/internal/cache/DistTXState.java       |  7 ++---
 .../geode/internal/cache/GemFireCacheImpl.java  | 27 ++++++++++----------
 .../NewDeclarativeIndexCreationJUnitTest.java   |  3 ---
 4 files changed, 22 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/f1b14b0d/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
index 6920311..44baa09 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionManager.java
@@ -263,7 +263,11 @@ public class DistributionManager implements DM {
   /** The id of this distribution manager */
   final protected InternalDistributedMember myid;
 
-  /** The distribution manager type of this dm; set in its constructor. */
+  /**
+   * The distribution manager type of this dm; set in its constructor.
+   * </p>
+   * TODO: change this to use an enum
+   */
   private final int dmType;
 
   /**

http://git-wip-us.apache.org/repos/asf/geode/blob/f1b14b0d/geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java b/geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java
index 226ffa6..8e2e618 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/DistTXState.java
@@ -45,14 +45,15 @@ import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.internal.offheap.annotations.Released;
 
 /**
- * TxState on a datanode VM
+ * TxState on a data node VM
  * 
  * 
  */
 public class DistTXState extends TXState {
 
-  public static Runnable internalBeforeApplyChanges;
-  public static Runnable internalBeforeNonTXBasicPut;
+  public static Runnable internalBeforeApplyChanges; // TODO: cleanup this test hook
+  public static Runnable internalBeforeNonTXBasicPut; // TODO: cleanup this test hook
+
   private boolean updatingTxStateDuringPreCommit = false;
 
   public DistTXState(TXStateProxy proxy, boolean onBehalfOfRemoteStub) {

http://git-wip-us.apache.org/repos/asf/geode/blob/f1b14b0d/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
index a181054..f3510da 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java
@@ -648,6 +648,7 @@ public class GemFireCacheImpl
     sb.append("; lockLease = ").append(this.lockLease);
     sb.append("; lockTimeout = ").append(this.lockTimeout);
     if (this.creationStack != null) {
+      // TODO: eliminate anonymous inner class and maybe move this to ExceptionUtils
       sb.append(System.lineSeparator()).append("Creation context:").append(System.lineSeparator());
       OutputStream os = new OutputStream() {
         @Override
@@ -2430,10 +2431,7 @@ public class GemFireCacheImpl
   @Override
   public Cache getReconnectedCache() {
     GemFireCacheImpl cache = GemFireCacheImpl.getInstance();
-    if (cache == null) {
-      return null;
-    }
-    if (cache == this || !cache.isInitialized()) {
+    if (cache == this || cache != null && !cache.isInitialized()) {
       cache = null;
     }
     return cache;
@@ -3074,8 +3072,11 @@ public class GemFireCacheImpl
         } catch (ExecutionException e) {
           throw new Error(LocalizedStrings.GemFireCache_UNEXPECTED_EXCEPTION.toLocalizedString(),
               e);
-        } catch (CancellationException ignore) {
+        } catch (CancellationException e) {
           // future was cancelled
+          if (logger.isTraceEnabled()) {
+            logger.trace("future cancelled", e);
+          }
         } finally {
           if (interrupted) {
             Thread.currentThread().interrupt();
@@ -4148,17 +4149,15 @@ public class GemFireCacheImpl
 
   @Override
   public QueryService getQueryService() {
-    if (isClient()) {
-      Pool pool = getDefaultPool();
-      if (pool == null) {
-        throw new IllegalStateException(
-            "Client cache does not have a default pool. Use getQueryService(String poolName) instead.");
-      } else {
-        return pool.getQueryService();
-      }
-    } else {
+    if (!isClient()) {
       return new DefaultQueryService(this);
     }
+    Pool defaultPool = getDefaultPool();
+    if (defaultPool == null) {
+      throw new IllegalStateException(
+          "Client cache does not have a default pool. Use getQueryService(String poolName) instead.");
+    }
+    return defaultPool.getQueryService();
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/geode/blob/f1b14b0d/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java b/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java
index e7f5c08..c15b812 100644
--- a/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/cache/query/internal/index/NewDeclarativeIndexCreationJUnitTest.java
@@ -158,8 +158,5 @@ public class NewDeclarativeIndexCreationJUnitTest {
     // TODO: refactoring GemFireCacheImpl.initializeDeclarativeCache requires change here
     assertThatThrownBy(() -> CacheFactory.create(ds)).isExactlyInstanceOf(CacheXmlException.class)
         .hasCauseInstanceOf(InternalGemFireException.class);
-
-    // hasCauseMessageContaining("CacheXmlParser::endIndex:Index creation attribute not correctly
-    // specified.");
   }
 }