Hawkin
/
arangodb
mirror of https://gitee.com/bigwinds/arangodb
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Bug fix/sparse graph register warning (#3569)

This commit is contained in:
Michael Hackstein 2017-11-10 14:33:15 +01:00 committed by Jan
parent bef52d7dc3
commit 2b0aa318ec
21 changed files with 197 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
CHANGELOG
View File

@ -1,6 +1,14 @@
devel
-----
* AQL: during a traversal if a vertex is not found. It will not print an ERROR to the log and continue
with a NULL value, but will register a warning at the query and continue with a NULL value.
The situation is not desired as an ERROR as ArangoDB can store edges pointing to non-existing
vertex which is perfectly valid, but it may be a n issue on the data model, so users
can directly see it on the query now and do not "by accident" have to check the LOG output.
* UI: fixed event cleanup in cluster shards view
* fixed issue #3618: Inconsistent behavior of OR statement with object bind parameters
* potential fix for issue #3562: Document WITHIN_RECTANGLE not found

12
arangod/Aql/ExecutionPlan.cpp
View File

@ -70,9 +70,9 @@ static uint64_t checkTraversalDepthValue(AstNode const* node) {
}
static std::unique_ptr<graph::BaseOptions> CreateTraversalOptions(
transaction::Methods* trx, AstNode const* direction,
aql::Query* query, AstNode const* direction,
AstNode const* optionsNode) {
auto options = std::make_unique<traverser::TraverserOptions>(trx);
auto options = std::make_unique<traverser::TraverserOptions>(query);
TRI_ASSERT(direction != nullptr);
TRI_ASSERT(direction->type == NODE_TYPE_DIRECTION);
@ -150,8 +150,8 @@ static std::unique_ptr<graph::BaseOptions> CreateTraversalOptions(
}
static std::unique_ptr<graph::BaseOptions> CreateShortestPathOptions(
arangodb::transaction::Methods* trx, AstNode const* node) {
auto options = std::make_unique<graph::ShortestPathOptions>(trx);
arangodb::aql::Query* query, AstNode const* node) {
auto options = std::make_unique<graph::ShortestPathOptions>(query);
if (node != nullptr && node->type == NODE_TYPE_OBJECT) {
size_t n = node->numMembers();
@ -727,7 +727,7 @@ ExecutionNode* ExecutionPlan::fromNodeTraversal(ExecutionNode* previous,
previous = calc;
}
auto options = CreateTraversalOptions(getAst()->query()->trx(), direction,
auto options = CreateTraversalOptions(getAst()->query(), direction,
node->getMember(3));
TRI_ASSERT(direction->type == NODE_TYPE_DIRECTION);
@ -809,7 +809,7 @@ ExecutionNode* ExecutionPlan::fromNodeShortestPath(ExecutionNode* previous,
parseTraversalVertexNode(previous, node->getMember(2));
AstNode const* graph = node->getMember(3);
auto options = CreateShortestPathOptions(getAst()->query()->trx(), node->getMember(4));
auto options = CreateShortestPathOptions(getAst()->query(), node->getMember(4));
// First create the node
auto spNode = new ShortestPathNode(this, nextId(), _ast->query()->vocbase(),

2
arangod/Aql/GraphNode.cpp
View File

@ -362,7 +362,7 @@ GraphNode::GraphNode(ExecutionPlan* plan,
"graph options have to be a json-object.");
}
_options = BaseOptions::createOptionsFromSlice(
_plan->getAst()->query()->trx(), opts);
_plan->getAst()->query(), opts);
}
/// @brief Internal constructor to clone the node.

6
arangod/Aql/Query.h
View File

@ -85,7 +85,7 @@ class Query {
std::shared_ptr<arangodb::velocypack::Builder> const& queryStruct,
std::shared_ptr<arangodb::velocypack::Builder> const& options, QueryPart);
~Query();
virtual ~Query();
/// @brief clone a query
/// note: as a side-effect, this will also create and start a transaction for
@ -176,7 +176,7 @@ class Query {
void registerErrorCustom(int, char const*);
/// @brief register a warning
void registerWarning(int, char const* = nullptr);
virtual void registerWarning(int, char const* = nullptr);
void prepare(QueryRegistry*, uint64_t queryHash);
@ -208,7 +208,7 @@ class Query {
void releaseEngine();
/// @brief return the transaction, if prepared
inline transaction::Methods* trx() { return _trx; }
virtual transaction::Methods* trx() { return _trx; }
/// @brief get the plan for the query
ExecutionPlan* plan() const { return _plan.get(); }

15
arangod/Graph/BaseOptions.cpp
View File

@ -160,23 +160,25 @@ double BaseOptions::LookupInfo::estimateCost(size_t& nrItems) const {
}
std::unique_ptr<BaseOptions> BaseOptions::createOptionsFromSlice(
transaction::Methods* trx, VPackSlice const& definition) {
arangodb::aql::Query* query, VPackSlice const& definition) {
VPackSlice type = definition.get("type");
if (type.isString() && type.isEqualString("shortestPath")) {
return std::make_unique<ShortestPathOptions>(trx, definition);
return std::make_unique<ShortestPathOptions>(query, definition);
}
return std::make_unique<TraverserOptions>(trx, definition);
return std::make_unique<TraverserOptions>(query, definition);
}
BaseOptions::BaseOptions(transaction::Methods* trx)
BaseOptions::BaseOptions(arangodb::aql::Query* query)
: _ctx(new aql::FixedVarExpressionContext()),
_trx(trx),
_query(query),
_trx(query->trx()),
_tmpVar(nullptr),
_isCoordinator(arangodb::ServerState::instance()->isCoordinator()),
_cache(nullptr) {}
BaseOptions::BaseOptions(BaseOptions const& other)
: _ctx(new aql::FixedVarExpressionContext()),
_query(other._query),
_trx(other._trx),
_tmpVar(nullptr),
_isCoordinator(arangodb::ServerState::instance()->isCoordinator()),
@ -188,6 +190,7 @@ BaseOptions::BaseOptions(BaseOptions const& other)
BaseOptions::BaseOptions(arangodb::aql::Query* query, VPackSlice info,
VPackSlice collections)
: _ctx(new aql::FixedVarExpressionContext()),
_query(query),
_trx(query->trx()),
_tmpVar(nullptr),
_isCoordinator(arangodb::ServerState::instance()->isCoordinator()),
@ -443,5 +446,5 @@ void BaseOptions::activateCache(
std::unordered_map<ServerID, traverser::TraverserEngineID> const* engines) {
// Do not call this twice.
TRI_ASSERT(_cache == nullptr);
_cache.reset(cacheFactory::CreateCache(_trx, enableDocumentCache, engines));
_cache.reset(cacheFactory::CreateCache(_query, enableDocumentCache, engines));
}

6
arangod/Graph/BaseOptions.h
View File

@ -80,9 +80,9 @@ struct BaseOptions {
public:
static std::unique_ptr<BaseOptions> createOptionsFromSlice(
transaction::Methods* trx, arangodb::velocypack::Slice const& definition);
arangodb::aql::Query* query, arangodb::velocypack::Slice const& definition);
explicit BaseOptions(transaction::Methods* trx);
explicit BaseOptions(arangodb::aql::Query* query);
/// @brief This copy constructor is only working during planning phase.
/// After planning this node should not be copied anywhere.
@ -161,6 +161,8 @@ struct BaseOptions {
std::vector<LookupInfo>&);
protected:
aql::Query* _query;
transaction::Methods* _trx;
/// @brief Lookup info to find all edges fulfilling the base conditions

14
arangod/Graph/ClusterTraverserCache.cpp
View File

@ -23,6 +23,7 @@
#include "ClusterTraverserCache.h"
#include "Aql/AqlValue.h"
#include "Aql/Query.h"
#include "Basics/StringRef.h"
#include "Basics/VelocyPackHelper.h"
#include "Cluster/ServerState.h"
@ -38,9 +39,9 @@ using namespace arangodb::basics;
using namespace arangodb::graph;
ClusterTraverserCache::ClusterTraverserCache(
transaction::Methods* trx,
aql::Query* query,
std::unordered_map<ServerID, traverser::TraverserEngineID> const* engines)
: TraverserCache(trx), _engines(engines) {}
: TraverserCache(query), _engines(engines) {}
VPackSlice ClusterTraverserCache::lookupToken(EdgeDocumentToken const& token) {
return VPackSlice(token.vpack());
@ -59,7 +60,10 @@ aql::AqlValue ClusterTraverserCache::fetchVertexAqlResult(StringRef id) {
TRI_ASSERT(ServerState::instance()->isCoordinator());
auto it = _cache.find(id);
if (it == _cache.end()) {
LOG_TOPIC(ERR, Logger::GRAPHS) << __FUNCTION__ << " vertex '" << id.toString() << "' not found";
// Register a warning. It is okay though but helps the user
std::string msg = "vertex '" + id.toString() + "' not found";
_query->registerWarning(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND, msg.c_str());
// Document not found return NULL
return aql::AqlValue(aql::AqlValueHintNull());
}
@ -78,7 +82,9 @@ void ClusterTraverserCache::insertVertexIntoResult(StringRef id,
VPackBuilder& result) {
auto it = _cache.find(id);
if (it == _cache.end()) {
LOG_TOPIC(ERR, Logger::GRAPHS) << __FUNCTION__ << " vertex '" << id.toString() << "' not found";
// Register a warning. It is okay though but helps the user
std::string msg = "vertex '" + id.toString() + "' not found";
_query->registerWarning(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND, msg.c_str());
// Document not found append NULL
result.add(VelocyPackHelper::NullValue());
} else {

3
arangod/Graph/ClusterTraverserCache.h
View File

@ -49,7 +49,7 @@ namespace graph {
class ClusterTraverserCache : public TraverserCache {
public:
ClusterTraverserCache(
transaction::Methods* trx,
aql::Query* query,
std::unordered_map<ServerID, traverser::TraverserEngineID> const*
engines);
@ -100,6 +100,7 @@ class ClusterTraverserCache : public TraverserCache {
}
private:
/// @brief link by _id into our data dump
std::unordered_map<StringRef, arangodb::velocypack::Slice> _cache;
/// @brief dump for our edge and vertex documents

8
arangod/Graph/ShortestPathOptions.cpp
View File

@ -39,17 +39,17 @@ using namespace arangodb::basics;
using namespace arangodb::graph;
using namespace arangodb::traverser;
ShortestPathOptions::ShortestPathOptions(transaction::Methods* trx)
: BaseOptions(trx),
ShortestPathOptions::ShortestPathOptions(aql::Query* query)
: BaseOptions(query),
direction("outbound"),
weightAttribute(""),
defaultWeight(1),
bidirectional(true),
multiThreaded(true) {}
ShortestPathOptions::ShortestPathOptions(transaction::Methods* trx,
ShortestPathOptions::ShortestPathOptions(aql::Query* query,
VPackSlice const& info)
: BaseOptions(trx),
: BaseOptions(query),
direction("outbound"),
weightAttribute(""),
defaultWeight(1),

8
arangod/Graph/ShortestPathOptions.h
View File

@ -33,10 +33,6 @@ class ExecutionPlan;
class Query;
}
namespace transaction {
class Methods;
}
namespace velocypack {
class Builder;
class Slice;
@ -55,9 +51,9 @@ struct ShortestPathOptions : public BaseOptions {
arangodb::velocypack::Builder startBuilder;
arangodb::velocypack::Builder endBuilder;
explicit ShortestPathOptions(transaction::Methods* trx);
explicit ShortestPathOptions(aql::Query* query);
ShortestPathOptions(transaction::Methods* trx,
ShortestPathOptions(aql::Query* query,
arangodb::velocypack::Slice const& info);
// @brief DBServer-constructor used by TraverserEngines

11
arangod/Graph/TraverserCache.cpp
View File

@ -27,6 +27,7 @@
#include "Basics/VelocyPackHelper.h"
#include "Aql/AqlValue.h"
#include "Aql/Query.h"
#include "Cluster/ServerState.h"
#include "Graph/EdgeDocumentToken.h"
#include "Logger/Logger.h"
@ -41,9 +42,10 @@
using namespace arangodb;
using namespace arangodb::graph;
TraverserCache::TraverserCache(transaction::Methods* trx)
TraverserCache::TraverserCache(aql::Query* query)
: _mmdr(new ManagedDocumentResult{}),
_trx(trx), _insertedDocuments(0),
_query(query),
_trx(query->trx()), _insertedDocuments(0),
_filteredDocuments(0),
_stringHeap(new StringHeap{4096}) /* arbitrary block-size may be adjusted for performance */ {
}
@ -90,6 +92,11 @@ VPackSlice TraverserCache::lookupInCollection(StringRef id) {
return VPackSlice(_mmdr->vpack());
} else if (res.is(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND)) {
++_insertedDocuments;
// Register a warning. It is okay though but helps the user
std::string msg = "vertex '" + id.toString() + "' not found";
_query->registerWarning(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND, msg.c_str());
// This is expected, we may have dangling edges. Interpret as NULL
return basics::VelocyPackHelper::NullValue();
} else {

8
arangod/Graph/TraverserCache.h
View File

@ -41,6 +41,7 @@ class Slice;
namespace aql {
struct AqlValue;
class Query;
}
namespace graph {
@ -55,7 +56,7 @@ struct EdgeDocumentToken;
class TraverserCache {
public:
explicit TraverserCache(transaction::Methods* trx);
explicit TraverserCache(aql::Query* query);
virtual ~TraverserCache();
@ -130,6 +131,11 @@ class TraverserCache {
//////////////////////////////////////////////////////////////////////////////
std::unique_ptr<ManagedDocumentResult> _mmdr;
//////////////////////////////////////////////////////////////////////////////
/// @brief Query used to register warnings to.
//////////////////////////////////////////////////////////////////////////////
arangodb::aql::Query* _query;
//////////////////////////////////////////////////////////////////////////////
/// @brief Transaction to access data, This class is NOT responsible for it.
//////////////////////////////////////////////////////////////////////////////

8
arangod/Graph/TraverserCacheFactory.cpp
View File

@ -35,13 +35,13 @@ using namespace arangodb::traverser;
using namespace arangodb::graph::cacheFactory;
TraverserCache* cacheFactory::CreateCache(
arangodb::transaction::Methods* trx, bool activateDocumentCache,
arangodb::aql::Query* query, bool activateDocumentCache,
std::unordered_map<ServerID, traverser::TraverserEngineID> const* engines) {
if (ServerState::instance()->isCoordinator()) {
return new ClusterTraverserCache(trx, engines);
return new ClusterTraverserCache(query, engines);
}
if (activateDocumentCache) {
return new TraverserDocumentCache(trx);
return new TraverserDocumentCache(query);
}
return new TraverserCache(trx);
return new TraverserCache(query);
}

6
arangod/Graph/TraverserCacheFactory.h
View File

@ -28,15 +28,15 @@
#include "Cluster/TraverserEngineRegistry.h"
namespace arangodb {
namespace transaction {
class Methods;
namespace aql {
class Query;
}
namespace graph {
class TraverserCache;
namespace cacheFactory {
TraverserCache* CreateCache(
arangodb::transaction::Methods* trx, bool activateDocumentCache,
arangodb::aql::Query* query, bool activateDocumentCache,
std::unordered_map<ServerID, traverser::TraverserEngineID> const* engines);
} // namespace cacheFactory

4
arangod/Graph/TraverserDocumentCache.cpp
View File

@ -41,8 +41,8 @@
using namespace arangodb;
using namespace arangodb::graph;
TraverserDocumentCache::TraverserDocumentCache(transaction::Methods* trx)
: TraverserCache(trx), _cache(nullptr) {
TraverserDocumentCache::TraverserDocumentCache(aql::Query* query)
: TraverserCache(query), _cache(nullptr) {
auto cacheManager = CacheManagerFeature::MANAGER;
TRI_ASSERT(cacheManager != nullptr);
_cache = cacheManager->createCache(cache::CacheType::Plain);

2
arangod/Graph/TraverserDocumentCache.h
View File

@ -37,7 +37,7 @@ namespace graph {
class TraverserDocumentCache : public TraverserCache {
public:
explicit TraverserDocumentCache(transaction::Methods* trx);
explicit TraverserDocumentCache(aql::Query* query);
~TraverserDocumentCache();

10
arangod/Graph/TraverserOptions.cpp
View File

@ -40,8 +40,8 @@ using namespace arangodb::transaction;
using namespace arangodb::traverser;
using VPackHelper = arangodb::basics::VelocyPackHelper;
TraverserOptions::TraverserOptions(transaction::Methods* trx)
: BaseOptions(trx),
TraverserOptions::TraverserOptions(aql::Query* query)
: BaseOptions(query),
_baseVertexExpression(nullptr),
_traverser(nullptr),
minDepth(1),
@ -50,9 +50,9 @@ TraverserOptions::TraverserOptions(transaction::Methods* trx)
uniqueVertices(UniquenessLevel::NONE),
uniqueEdges(UniquenessLevel::PATH) {}
TraverserOptions::TraverserOptions(transaction::Methods* trx,
TraverserOptions::TraverserOptions(aql::Query* query,
VPackSlice const& obj)
: BaseOptions(trx),
: BaseOptions(query),
_baseVertexExpression(nullptr),
_traverser(nullptr),
minDepth(1),
@ -238,7 +238,7 @@ arangodb::traverser::TraverserOptions::TraverserOptions(
arangodb::traverser::TraverserOptions::TraverserOptions(
TraverserOptions const& other)
: BaseOptions(other.trx()),
: BaseOptions(other._query),
_baseVertexExpression(nullptr),
_traverser(nullptr),
minDepth(other.minDepth),

4
arangod/Graph/TraverserOptions.h
View File

@ -82,9 +82,9 @@ struct TraverserOptions : public graph::BaseOptions {
UniquenessLevel uniqueEdges;
explicit TraverserOptions(transaction::Methods* trx);
explicit TraverserOptions(aql::Query* query);
TraverserOptions(transaction::Methods* trx, arangodb::velocypack::Slice const& definition);
TraverserOptions(aql::Query* query, arangodb::velocypack::Slice const& definition);
TraverserOptions(arangodb::aql::Query*, arangodb::velocypack::Slice,
arangodb::velocypack::Slice);

9
js/server/tests/aql/aql-general-graph.js
View File

@ -1,5 +1,5 @@
/*jshint globalstrict:false, strict:false, maxlen: 500 */
/*global assertEqual, assertTrue */
/*global assertEqual, assertTrue, AQL_EXECUTE */
////////////////////////////////////////////////////////////////////////////////
/// @brief tests for query language, graph functions
@ -30,6 +30,7 @@
var jsunity = require("jsunity");
var db = require("@arangodb").db;
var errors = require("internal").errors;
var graph = require("@arangodb/general-graph");
var helper = require("@arangodb/aql-helper");
var getQueryResults = helper.getQueryResults;
@ -1935,7 +1936,8 @@ function ahuacatlQueryShortestPathTestSuite() {
LET source = "${v1}/A"
LET target = "${v1}/F"
FOR v, e IN OUTBOUND SHORTEST_PATH source TO target GRAPH "${graphName}" RETURN {v, e}`;
var actual = getQueryResults(query);
var full = AQL_EXECUTE(query);
var actual = full.json;
assertEqual(actual.length, 4);
assertEqual(actual[0].v._key, "A");
assertEqual(actual[0].e, null);
@ -1946,6 +1948,9 @@ function ahuacatlQueryShortestPathTestSuite() {
assertEqual(actual[2].e._to, v2 + "/D");
assertEqual(actual[3].v._key, "F");
assertEqual(actual[3].e.entfernung, 11);
// We expect one warning
assertEqual(full.warnings.length, 1);
assertEqual(full.warnings[0].code, errors.ERROR_ARANGO_DOCUMENT_NOT_FOUND.code);
}
};

1
tests/CMakeLists.txt
View File

@ -53,6 +53,7 @@ add_executable(
Cache/TransactionsWithBackingStore.cpp
Cluster/ClusterHelpersTest.cpp
Geo/georeg.cpp
Graph/ClusterTraverserCacheTest.cpp
Pregel/typedbuffer.cpp
RocksDBEngine/KeyTest.cpp
RocksDBEngine/IndexEstimatorTest.cpp

107
tests/Graph/ClusterTraverserCacheTest.cpp Normal file
View File

@ -0,0 +1,107 @@
////////////////////////////////////////////////////////////////////////////////
/// DISCLAIMER
///
/// Copyright 2017-2017 ArangoDB 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 ArangoDB GmbH, Cologne, Germany
///
/// @author Michael Hackstein
////////////////////////////////////////////////////////////////////////////////
#include "catch.hpp"
#include "fakeit.hpp"
#include "Aql/AqlValue.h"
#include "Aql/Query.h"
#include "Cluster/ServerState.h"
#include "Graph/ClusterTraverserCache.h"
#include "Transaction/Methods.h"
#include <velocypack/Builder.h>
#include <velocypack/Slice.h>
#include <velocypack/velocypack-aliases.h>
using namespace arangodb;
using namespace arangodb::aql;
using namespace arangodb::graph;
using namespace fakeit;
namespace arangodb {
namespace tests {
namespace cluster_traverser_cache_test {
TEST_CASE("ClusterTraverserCache", "[aql][cluster]") {
auto ss = ServerState::instance();
ss->setRole(ServerState::ROLE_COORDINATOR);
SECTION("it should return a NULL AQLValue if vertex not cached") {
std::unordered_map<ServerID, traverser::TraverserEngineID> engines;
std::string vertexId = "UnitTest/Vertex";
std::string expectedMessage = "vertex '" + vertexId + "' not found";
Mock<transaction::Methods> trxMock;
transaction::Methods& trx = trxMock.get();
Mock<Query> queryMock;
Query& query = queryMock.get();
When(Method(queryMock, trx)).AlwaysReturn(&trx);
When(Method(queryMock, registerWarning)).Do([&] (int code, char const* message) {
REQUIRE(code == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND);
REQUIRE(strcmp(message, expectedMessage.c_str()) == 0);
});
ClusterTraverserCache testee(&query, &engines);
// NOTE: we do not put anything into the cache, so we get null for any vertex
AqlValue val = testee.fetchVertexAqlResult(StringRef(vertexId));
REQUIRE(val.isNull(false));
Verify(Method(queryMock, registerWarning)).Exactly(1);
}
SECTION("it should insert a NULL VPack if vertex not cached") {
std::unordered_map<ServerID, traverser::TraverserEngineID> engines;
std::string vertexId = "UnitTest/Vertex";
std::string expectedMessage = "vertex '" + vertexId + "' not found";
Mock<transaction::Methods> trxMock;
transaction::Methods& trx = trxMock.get();
Mock<Query> queryMock;
Query& query = queryMock.get();
When(Method(queryMock, trx)).AlwaysReturn(&trx);
When(Method(queryMock, registerWarning)).Do([&] (int code, char const* message) {
REQUIRE(code == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND);
REQUIRE(strcmp(message, expectedMessage.c_str()) == 0);
});
VPackBuilder result;
ClusterTraverserCache testee(&query, &engines);
// NOTE: we do not put anything into the cache, so we get null for any vertex
testee.insertVertexIntoResult(StringRef(vertexId), result);
VPackSlice sl = result.slice();
REQUIRE(sl.isNull());
Verify(Method(queryMock, registerWarning)).Exactly(1);
}
}
} // cluster_traveser_cache_test
} // tests
} // arangodb