attempt to fix tests

arangodb · jsteemann · Dec 12, 2018 · Dec 11, 2018 · Dec 11, 2018 · Dec 11, 2018
commit a802c634e017e267a8f08bf760ff995e07cf39a8
diff --git a/arangod/VocBase/KeyGenerator.cpp b/arangod/VocBase/KeyGenerator.cpp
@@ -194,7 +194,9 @@ class TraditionalKeyGeneratorSingle final : public TraditionalKeyGenerator {
  public:
   /// @brief create the generator
   explicit TraditionalKeyGeneratorSingle(bool allowUserKeys) 
-    : TraditionalKeyGenerator(allowUserKeys), _lastValue(0) {}
+    : TraditionalKeyGenerator(allowUserKeys), _lastValue(0) {
+    TRI_ASSERT(!ServerState::instance()->isCoordinator());
+  }
 
   /// @brief build a VelocyPack representation of the generator in the builder
   void toVelocyPack(arangodb::velocypack::Builder& builder) const override {
@@ -252,11 +254,19 @@ class TraditionalKeyGeneratorSingle final : public TraditionalKeyGenerator {
 };
 
 /// @brief traditional key generator for a coordinator
+/// please note that coordinator-based key generators are frequently
+/// created and discarded, so ctor & dtor need to be very efficient.
+/// additionally, do not put any state into this object, as for the
+/// same logical collection the ClusterInfo may create many different 
+/// temporary LogicalCollection objects one after the other, which
+/// will also discard the collection's particular KeyGenerator object!
 class TraditionalKeyGeneratorCluster final : public TraditionalKeyGenerator {
  public:
   /// @brief create the generator
   explicit TraditionalKeyGeneratorCluster(bool allowUserKeys)
-    : TraditionalKeyGenerator(allowUserKeys) {}
+    : TraditionalKeyGenerator(allowUserKeys) {
+    TRI_ASSERT(ServerState::instance()->isCoordinator());
+  }
 
  private:
   /// @brief generate a key value (internal)
@@ -385,7 +395,9 @@ class PaddedKeyGeneratorSingle final : public PaddedKeyGenerator {
  public:
   /// @brief create the generator
   explicit PaddedKeyGeneratorSingle(bool allowUserKeys) 
-    : PaddedKeyGenerator(allowUserKeys), _lastValue(0) {}
+    : PaddedKeyGenerator(allowUserKeys), _lastValue(0) {
+    TRI_ASSERT(!ServerState::instance()->isCoordinator());
+  }
 
   /// @brief build a VelocyPack representation of the generator in the builder
   void toVelocyPack(arangodb::velocypack::Builder& builder) const override {
@@ -441,11 +453,19 @@ class PaddedKeyGeneratorSingle final : public PaddedKeyGenerator {
 };
 
 /// @brief padded key generator for a coordinator
+/// please note that coordinator-based key generators are frequently
+/// created and discarded, so ctor & dtor need to be very efficient.
+/// additionally, do not put any state into this object, as for the
+/// same logical collection the ClusterInfo may create many different 
+/// temporary LogicalCollection objects one after the other, which
+/// will also discard the collection's particular KeyGenerator object!
 class PaddedKeyGeneratorCluster final : public PaddedKeyGenerator {
  public:
   /// @brief create the generator
   explicit PaddedKeyGeneratorCluster(bool allowUserKeys)
-    : PaddedKeyGenerator(allowUserKeys) {}
+    : PaddedKeyGenerator(allowUserKeys) {
+    TRI_ASSERT(ServerState::instance()->isCoordinator());
+  }
 
  private:
   /// @brief generate a key value (internal)

diff --git a/arangod/VocBase/KeyGenerator.h b/arangod/VocBase/KeyGenerator.h
@@ -35,6 +35,14 @@ class Builder;
 class Slice;
 }
 
+/// generic key generator interface
+///
+/// please note that coordinator-based key generators are frequently
+/// created and discarded, so ctor & dtor need to be very efficient.
+/// additionally, do not put any state into this object, as for the
+/// same logical collection the ClusterInfo may create many different 
+/// temporary LogicalCollection objects one after the other, which
+/// will also discard the collection's particular KeyGenerator object!
 class KeyGenerator {
   KeyGenerator(KeyGenerator const&) = delete;
   KeyGenerator& operator=(KeyGenerator const&) = delete;

diff --git a/arangod/VocBase/LogicalCollection.h b/arangod/VocBase/LogicalCollection.h
@@ -75,6 +75,14 @@ class ChecksumResult : public Result {
   velocypack::Builder _builder;
 };
 
+/// please note that coordinator-based logical collections are frequently
+/// created and discarded, so ctor & dtor need to be as efficient as possible.
+/// additionally, do not put any volatile state into this object in the coordinator, 
+/// as the ClusterInfo may create many different temporary physical LogicalCollection 
+/// objects (one after the other) even for the same "logical" LogicalCollection.
+/// this which will also discard the collection's volatile state each time!
+/// all state of a LogicalCollection in the coordinator case needs to be derived
+/// from the JSON info in the agency's plan entry for the collection...
 class LogicalCollection : public LogicalDataSource {
   friend struct ::TRI_vocbase_t;
 
@@ -424,4 +432,4 @@ class LogicalCollection : public LogicalDataSource {
 
 }  // namespace arangodb
 
-#endif
+#endif
diff --git a/tests/js/common/shell/shell-keygen.js b/tests/js/common/shell/shell-keygen.js
@@ -273,7 +273,9 @@ function TraditionalSuite () {
         lastKey = key;
       }
 
-      assertEqual(lastKey, c.properties().keyOptions.lastValue);
+      if (!cluster || !cluster.isCluster || !cluster.isCluster()) {
+        assertEqual(lastKey, c.properties().keyOptions.lastValue);
+      }
     },
 
     testPaddedTracking : function () {
@@ -293,8 +295,10 @@ function TraditionalSuite () {
         lastKey = key;
       }
 
-      let hex = c.properties().keyOptions.lastValue.toString(16);
-      assertEqual(lastKey, Array(16 + 1 - hex.length).join("0") + hex);
+      if (!cluster || !cluster.isCluster || !cluster.isCluster()) {
+        let hex = c.properties().keyOptions.lastValue.toString(16);
+        assertEqual(lastKey, Array(16 + 1 - hex.length).join("0") + hex);
+      }
     }
 
   };