Bug fix 3.5/honor sharding restrictions (#10140)

* honor sharding restrictions also when creating a graph with existing
collections

* add test

* additional validation for startup parameters

* fixed logIDs

arangod/Cluster/ClusterFeature.cpp

@@ -193,6 +193,15 @@ void ClusterFeature::validateOptions(std::shared_ptr<ProgramOptions> options) {
     _defaultReplicationFactor = _minReplicationFactor;
   }
   
+  if (!options->processingResult().touched("cluster.system-replication-factor")) {
+    // no system replication factor set. now make sure it is between min and max
+    if (_systemReplicationFactor > _maxReplicationFactor) {
+      _systemReplicationFactor = _maxReplicationFactor;
+    } else if (_systemReplicationFactor < _minReplicationFactor) {
+      _systemReplicationFactor = _minReplicationFactor;
+    }
+  }
+  
   if (_defaultReplicationFactor == 0) {
     // default replication factor must not be 0
     LOG_TOPIC("fc8a9", FATAL, arangodb::Logger::CLUSTER)
@@ -200,10 +209,17 @@ void ClusterFeature::validateOptions(std::shared_ptr<ProgramOptions> options) {
     FATAL_ERROR_EXIT();
   }
   
+  if (_systemReplicationFactor == 0) {
+    // default replication factor must not be 0
+    LOG_TOPIC("46935", FATAL, arangodb::Logger::CLUSTER)
+        << "Invalid value for `--cluster.system-replication-factor`. The value must be at least 1";
+    FATAL_ERROR_EXIT();
+  }
+  
   if (_defaultReplicationFactor > 0 &&
       _maxReplicationFactor > 0 && 
       _defaultReplicationFactor > _maxReplicationFactor) {
-    LOG_TOPIC("6cf0c", FATAL, arangodb::Logger::CLUSTER)
+    LOG_TOPIC("5af7e", FATAL, arangodb::Logger::CLUSTER)
         << "Invalid value for `--cluster.default-replication-factor`. Must not be higher than `--cluster.max-replication-factor`";
     FATAL_ERROR_EXIT();
   }
@@ -214,6 +230,21 @@ void ClusterFeature::validateOptions(std::shared_ptr<ProgramOptions> options) {
         << "Invalid value for `--cluster.default-replication-factor`. Must not be lower than `--cluster.min-replication-factor`";
     FATAL_ERROR_EXIT();
   }
+  
+  if (_systemReplicationFactor > 0 &&
+      _maxReplicationFactor > 0 && 
+      _systemReplicationFactor > _maxReplicationFactor) {
+    LOG_TOPIC("6cf0c", FATAL, arangodb::Logger::CLUSTER)
+        << "Invalid value for `--cluster.system-replication-factor`. Must not be higher than `--cluster.max-replication-factor`";
+    FATAL_ERROR_EXIT();
+  }
+  
+  if (_systemReplicationFactor > 0 &&
+      _systemReplicationFactor < _minReplicationFactor) {
+    LOG_TOPIC("dfc38", FATAL, arangodb::Logger::CLUSTER)
+        << "Invalid value for `--cluster.system-replication-factor`. Must not be lower than `--cluster.min-replication-factor`";
+    FATAL_ERROR_EXIT();
+  }
 
   // check if the cluster is enabled
   _enableCluster = !_agencyEndpoints.empty();

arangod/Graph/GraphManager.cpp

@@ -970,6 +970,13 @@ ResultT<std::unique_ptr<Graph>> GraphManager::buildGraphFromInput(std::string co
                                                                   VPackSlice input) const {
   try {
     TRI_ASSERT(input.isObject());
+    if (ServerState::instance()->isCoordinator()) {
+      // validate numberOfShards and replicationFactor
+      Result res = ShardingInfo::validateShardsAndReplicationFactor(input.get("options"));
+      if (res.fail()) {
+        return res;
+      }
+    }
     return Graph::fromUserInput(graphName, input, input.get(StaticStrings::GraphOptions));
   } catch (arangodb::basics::Exception const& e) {
     return Result{e.code(), e.message()};

tests/js/server/shell/shell-general-graph-creation-cluster.js

@@ -78,6 +78,18 @@ function GeneralGraphClusterCreationSuite() {
         assertEqual(ERRORS.ERROR_CLUSTER_TOO_MANY_SHARDS.code, err.errorNum);
       }
     },
+    
+    testCreateMoreShardsThanAllowedExistingCollections : function () {
+      db._create(vn);
+      db._createEdgeCollection(en);
+      let max = internal.maxNumberOfShards;
+      try {
+        graph._create(gn, edgeDef, null, { numberOfShards: max + 1 });
+        fail();
+      } catch (err) {
+        assertEqual(ERRORS.ERROR_CLUSTER_TOO_MANY_SHARDS.code, err.errorNum);
+      }
+    },
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief test replicationFactor
@@ -117,6 +129,28 @@ function GeneralGraphClusterCreationSuite() {
         assertEqual(ERRORS.ERROR_CLUSTER_INSUFFICIENT_DBSERVERS.code, err.errorNum);
       }
     },
+    
+    testMaxReplicationFactorExistingCollection : function () {
+      db._create(vn);
+      db._createEdgeCollection(en);
+      let max = internal.maxReplicationFactor;
+      try {
+        let myGraph = graph._create(gn, edgeDef, null, { replicationFactor: max });
+        let properties = db._graphs.document(gn);
+        assertEqual(max, properties.replicationFactor);
+        
+        graph._drop(gn, true);
+        try {
+          graph._create(gn, edgeDef, null, { replicationFactor: max + 1 });
+          fail();
+        } catch (err) {
+          assertEqual(ERRORS.ERROR_BAD_PARAMETER.code, err.errorNum);
+        }
+      } catch (err) {
+        // if creation fails, then it must have been exactly this error
+        assertEqual(ERRORS.ERROR_CLUSTER_INSUFFICIENT_DBSERVERS.code, err.errorNum);
+      }
+    },
 
   };
 }