From 7e524f9af9b7d790682e7fde0c7a48f3cb990633 Mon Sep 17 00:00:00 2001 From: Jan Date: Tue, 1 Oct 2019 22:55:58 +0200 Subject: [PATCH] honor restrictions when creating graphs (#10136) --- arangod/Graph/GraphManager.cpp | 8 ++ arangod/Sharding/ShardingInfo.cpp | 60 +++++---- .../shell-general-graph-creation-cluster.js | 126 ++++++++++++++++++ 3 files changed, 165 insertions(+), 29 deletions(-) create mode 100644 tests/js/server/shell/shell-general-graph-creation-cluster.js diff --git a/arangod/Graph/GraphManager.cpp b/arangod/Graph/GraphManager.cpp index ae92d4f5b9..450116d911 100644 --- a/arangod/Graph/GraphManager.cpp +++ b/arangod/Graph/GraphManager.cpp @@ -43,6 +43,7 @@ #include "Cluster/ServerState.h" #include "Graph/Graph.h" #include "RestServer/QueryRegistryFeature.h" +#include "Sharding/ShardingInfo.h" #include "Transaction/Methods.h" #include "Transaction/StandaloneContext.h" #include "Transaction/V8Context.h" @@ -92,6 +93,13 @@ OperationResult GraphManager::createCollection(std::string const& name, TRI_col_ bool waitForSync, VPackSlice options) { TRI_ASSERT(colType == TRI_COL_TYPE_DOCUMENT || colType == TRI_COL_TYPE_EDGE); + if (ServerState::instance()->isCoordinator()) { + Result res = ShardingInfo::validateShardsAndReplicationFactor(options); + if (res.fail()) { + return OperationResult(res); + } + } + auto res = arangodb::methods::Collections::create( // create collection ctx()->vocbase(), // collection vocbase name, // collection name diff --git a/arangod/Sharding/ShardingInfo.cpp b/arangod/Sharding/ShardingInfo.cpp index 2c8e3184e7..2d331a1716 100644 --- a/arangod/Sharding/ShardingInfo.cpp +++ b/arangod/Sharding/ShardingInfo.cpp @@ -475,39 +475,41 @@ int ShardingInfo::getResponsibleShard(arangodb::velocypack::Slice slice, bool do Result ShardingInfo::validateShardsAndReplicationFactor(arangodb::velocypack::Slice slice) { Result res; - - auto const* cl = application_features::ApplicationServer::getFeature("Cluster"); + + if (slice.isObject()) { + auto const* cl = application_features::ApplicationServer::getFeature("Cluster"); - auto numberOfShardsSlice = slice.get(StaticStrings::NumberOfShards); - if (numberOfShardsSlice.isNumber()) { - uint32_t const maxNumberOfShards = cl->maxNumberOfShards(); - uint32_t numberOfShards = numberOfShardsSlice.getNumber(); - if (maxNumberOfShards > 0 && - numberOfShards > maxNumberOfShards) { - res.reset(TRI_ERROR_CLUSTER_TOO_MANY_SHARDS, - std::string("too many shards. maximum number of shards is ") + std::to_string(maxNumberOfShards)); + auto numberOfShardsSlice = slice.get(StaticStrings::NumberOfShards); + if (numberOfShardsSlice.isNumber()) { + uint32_t const maxNumberOfShards = cl->maxNumberOfShards(); + uint32_t numberOfShards = numberOfShardsSlice.getNumber(); + if (maxNumberOfShards > 0 && + numberOfShards > maxNumberOfShards) { + res.reset(TRI_ERROR_CLUSTER_TOO_MANY_SHARDS, + std::string("too many shards. maximum number of shards is ") + std::to_string(maxNumberOfShards)); + } } - } - auto replicationFactorSlice = slice.get(StaticStrings::ReplicationFactor); - if (replicationFactorSlice.isNumber()) { - uint32_t const minReplicationFactor = cl->minReplicationFactor(); - uint32_t const maxReplicationFactor = cl->maxReplicationFactor(); - - int64_t replicationFactorProbe = replicationFactorSlice.getNumber(); - if (replicationFactorProbe <= 0) { - res.reset(TRI_ERROR_BAD_PARAMETER, "invalid value for replicationFactor"); - } else { - uint32_t replicationFactor = replicationFactorSlice.getNumber(); + auto replicationFactorSlice = slice.get(StaticStrings::ReplicationFactor); + if (replicationFactorSlice.isNumber()) { + uint32_t const minReplicationFactor = cl->minReplicationFactor(); + uint32_t const maxReplicationFactor = cl->maxReplicationFactor(); - if (replicationFactor > maxReplicationFactor && - maxReplicationFactor > 0) { - res.reset(TRI_ERROR_BAD_PARAMETER, - std::string("replicationFactor must not be higher than maximum allowed replicationFactor (") + std::to_string(maxReplicationFactor) + ")"); - } else if (replicationFactor < minReplicationFactor && - minReplicationFactor > 0) { - res.reset(TRI_ERROR_BAD_PARAMETER, - std::string("replicationFactor must not be lower than minimum allowed replicationFactor (") + std::to_string(minReplicationFactor) + ")"); + int64_t replicationFactorProbe = replicationFactorSlice.getNumber(); + if (replicationFactorProbe <= 0) { + res.reset(TRI_ERROR_BAD_PARAMETER, "invalid value for replicationFactor"); + } else { + uint32_t replicationFactor = replicationFactorSlice.getNumber(); + + if (replicationFactor > maxReplicationFactor && + maxReplicationFactor > 0) { + res.reset(TRI_ERROR_BAD_PARAMETER, + std::string("replicationFactor must not be higher than maximum allowed replicationFactor (") + std::to_string(maxReplicationFactor) + ")"); + } else if (replicationFactor < minReplicationFactor && + minReplicationFactor > 0) { + res.reset(TRI_ERROR_BAD_PARAMETER, + std::string("replicationFactor must not be lower than minimum allowed replicationFactor (") + std::to_string(minReplicationFactor) + ")"); + } } } } diff --git a/tests/js/server/shell/shell-general-graph-creation-cluster.js b/tests/js/server/shell/shell-general-graph-creation-cluster.js new file mode 100644 index 0000000000..5c5f0f81f1 --- /dev/null +++ b/tests/js/server/shell/shell-general-graph-creation-cluster.js @@ -0,0 +1,126 @@ +/*jshint globalstrict:false, strict:false */ +/*global assertEqual, assertTrue, assertFalse, fail */ + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test the general-graph class +/// +/// @file +/// +/// DISCLAIMER +/// +/// Copyright 2010-2014 triagens GmbH, Cologne, Germany +/// +/// Licensed 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. +/// +/// Copyright holder is triAGENS GmbH, Cologne, Germany +/// +/// @author Jan Steemann +/// @author Copyright 2019, triAGENS GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +var jsunity = require("jsunity"); +var arangodb = require("@arangodb"); +var db = arangodb.db; +var graph = require("@arangodb/general-graph"); +var ERRORS = arangodb.errors; +let internal = require("internal"); + +function GeneralGraphClusterCreationSuite() { + 'use strict'; + const vn = "UnitTestVertices"; + const en = "UnitTestEdges"; + const gn = "UnitTestGraph"; + const edgeDef = [graph._relation(en, vn, vn)]; + + return { + + setUp: function () { + try { + graph._drop(gn, true); + } catch (ignore) { + } + }, + + tearDown: function () { + try { + graph._drop(gn, true); + } catch (ignore) { + } + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test numberOfShards +//////////////////////////////////////////////////////////////////////////////// + + testCreateAsManyShardsAsAllowed : function () { + let max = internal.maxNumberOfShards; + let myGraph = graph._create(gn, edgeDef, null, { numberOfShards: max }); + let properties = db._graphs.document(gn); + assertEqual(max, properties.numberOfShards); + }, + + testCreateMoreShardsThanAllowed : function () { + 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 +//////////////////////////////////////////////////////////////////////////////// + + testMinReplicationFactor : function () { + let min = internal.minReplicationFactor; + let myGraph = graph._create(gn, edgeDef, null, { replicationFactor: min }); + let properties = db._graphs.document(gn); + assertEqual(min, properties.replicationFactor); + + graph._drop(gn, true); + try { + graph._create(gn, edgeDef, null, { replicationFactor: min - 1 }); + fail(); + } catch (err) { + assertEqual(ERRORS.ERROR_BAD_PARAMETER.code, err.errorNum); + } + }, + + testMaxReplicationFactor : function () { + 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); + } + }, + + }; +} + +jsunity.run(GeneralGraphClusterCreationSuite); + +return jsunity.done();