From 268dc3c09739dd4b5d0e122bcffa489ca1dee64b Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Mon, 21 Mar 2016 09:12:56 +0100 Subject: [PATCH 1/2] Fix geo index remove. --- arangod/Indexes/GeoIndex2.cpp | 49 ++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/arangod/Indexes/GeoIndex2.cpp b/arangod/Indexes/GeoIndex2.cpp index ffeb353f8f..b9ac5e6124 100644 --- a/arangod/Indexes/GeoIndex2.cpp +++ b/arangod/Indexes/GeoIndex2.cpp @@ -187,31 +187,50 @@ int GeoIndex2::remove(arangodb::Transaction*, TRI_doc_mptr_t const* doc, bool) { double latitude; double longitude; + bool ok = true; if (_variant == INDEX_GEO_INDIVIDUAL_LAT_LON) { VPackSlice lat = slice.get(_latitude); VPackSlice lon = slice.get(_longitude); - TRI_ASSERT(lat.isNumber()); - latitude = lat.getNumericValue(); - TRI_ASSERT(lon.isNumber()); - longitude = lon.getNumericValue(); + if (!lat.isNumber()) { + ok = false; + } else { + latitude = lat.getNumericValue(); + } + if (!lon.isNumber()) { + ok = false; + } else { + longitude = lon.getNumericValue(); + } } else { VPackSlice loc = slice.get(_location); - TRI_ASSERT(loc.isArray()); - TRI_ASSERT(loc.length() >= 2); - VPackSlice first = loc.at(0); - TRI_ASSERT(first.isNumber()); - VPackSlice second = loc.at(1); - TRI_ASSERT(second.isNumber()); - if (_geoJson) { - longitude = first.getNumericValue(); - latitude = second.getNumericValue(); + if (!loc.isArray() || loc.length() < 2) { + ok = false; } else { - latitude = first.getNumericValue(); - longitude = second.getNumericValue(); + VPackSlice first = loc.at(0); + if (!first.isNumber()) { + ok = false; + } + VPackSlice second = loc.at(1); + if (!second.isNumber()) { + ok = false; + } + if (ok) { + if (_geoJson) { + longitude = first.getNumericValue(); + latitude = second.getNumericValue(); + } else { + latitude = first.getNumericValue(); + longitude = second.getNumericValue(); + } + } } } + if (!ok) { + return TRI_ERROR_NO_ERROR; + } + GeoCoordinate gc; gc.latitude = latitude; gc.longitude = longitude; From 7896905ad769c1b74c1ab9782deeef314b61848d Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Mon, 21 Mar 2016 09:20:31 +0100 Subject: [PATCH 2/2] Fixed a multithreading bug for EdgeCollectionInfos. Threads shared the same Builder instance which is not possible. --- arangod/V8Server/V8Traverser.cpp | 25 ++++++++----------------- arangod/V8Server/V8Traverser.h | 11 ++--------- arangod/V8Server/v8-vocbase.cpp | 4 ++-- 3 files changed, 12 insertions(+), 28 deletions(-) diff --git a/arangod/V8Server/V8Traverser.cpp b/arangod/V8Server/V8Traverser.cpp index 6f7f1c5c80..6d2e2d1c2f 100644 --- a/arangod/V8Server/V8Traverser.cpp +++ b/arangod/V8Server/V8Traverser.cpp @@ -68,23 +68,13 @@ EdgeCollectionInfo::EdgeCollectionInfo(arangodb::Transaction* trx, std::string const& collectionName, WeightCalculatorFunction weighter) : _trx(trx), _collectionName(collectionName), _weighter(weighter) { - TRI_voc_cid_t cid = trx->resolver()->getCollectionIdLocal(collectionName); - if (cid == 0) { - THROW_ARANGO_EXCEPTION_FORMAT(TRI_ERROR_ARANGO_COLLECTION_NOT_FOUND, "'%s'", - collectionName.c_str()); - } - trx->addCollectionAtRuntime(cid); + trx->addCollectionAtRuntime(collectionName); if (!trx->isEdgeCollection(collectionName)) { THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_COLLECTION_TYPE_INVALID); } - - TRI_document_collection_t* documentCollection = trx->documentCollection(cid); - arangodb::EdgeIndex* edgeIndex = documentCollection->edgeIndex(); - TRI_ASSERT(edgeIndex != - nullptr); // Checked because collection is edge Collection. - _indexId = arangodb::basics::StringUtils::itoa(edgeIndex->id()); + _indexId = trx->edgeIndexHandle(collectionName); } //////////////////////////////////////////////////////////////////////////////// @@ -93,11 +83,12 @@ EdgeCollectionInfo::EdgeCollectionInfo(arangodb::Transaction* trx, std::shared_ptr EdgeCollectionInfo::getEdges( TRI_edge_direction_e direction, std::string const& vertexId) { - _searchBuilder.clear(); - EdgeIndex::buildSearchValue(direction, vertexId, _searchBuilder); +#warning Make this thread safe s.t. we only need 2 builders. + VPackBuilder searchBuilder; + EdgeIndex::buildSearchValue(direction, vertexId, searchBuilder); return _trx->indexScan(_collectionName, arangodb::Transaction::CursorType::INDEX, _indexId, - _searchBuilder.slice(), 0, UINT64_MAX, 1000, false); + searchBuilder.slice(), 0, UINT64_MAX, 1000, false); } //////////////////////////////////////////////////////////////////////////////// @@ -450,7 +441,7 @@ void NeighborsOptions::addCollectionRestriction(TRI_voc_cid_t cid) { //////////////////////////////////////////////////////////////////////////////// std::unique_ptr TRI_RunShortestPathSearch( - std::vector& collectionInfos, + std::vector const& collectionInfos, ShortestPathOptions& opts) { TRI_edge_direction_e forward; TRI_edge_direction_e backward; @@ -494,7 +485,7 @@ std::unique_ptr TRI_RunShortestPathSearch( std::unique_ptr TRI_RunSimpleShortestPathSearch( - std::vector& collectionInfos, + std::vector const& collectionInfos, arangodb::Transaction* trx, ShortestPathOptions& opts) { TRI_edge_direction_e forward; diff --git a/arangod/V8Server/V8Traverser.h b/arangod/V8Server/V8Traverser.h index 77b8736c14..9b314fb804 100644 --- a/arangod/V8Server/V8Traverser.h +++ b/arangod/V8Server/V8Traverser.h @@ -389,13 +389,6 @@ class EdgeCollectionInfo { std::string _indexId; - ////////////////////////////////////////////////////////////////////////////// - /// @brief VPackBuilder to build edge index search value in place. - /// Reused for every request. - ////////////////////////////////////////////////////////////////////////////// - - arangodb::velocypack::Builder _searchBuilder; - WeightCalculatorFunction _weighter; public: @@ -425,12 +418,12 @@ class EdgeCollectionInfo { //////////////////////////////////////////////////////////////////////////////// std::unique_ptr TRI_RunShortestPathSearch( - std::vector& collectionInfos, + std::vector const& collectionInfos, arangodb::traverser::ShortestPathOptions& opts); std::unique_ptr TRI_RunSimpleShortestPathSearch( - std::vector& collectionInfos, + std::vector const& collectionInfos, arangodb::Transaction*, arangodb::traverser::ShortestPathOptions& opts); diff --git a/arangod/V8Server/v8-vocbase.cpp b/arangod/V8Server/v8-vocbase.cpp index cfe412efaf..9d547cd842 100644 --- a/arangod/V8Server/v8-vocbase.cpp +++ b/arangod/V8Server/v8-vocbase.cpp @@ -2082,9 +2082,9 @@ static void JS_QueryShortestPath( if (opts.useEdgeFilter) { std::string errorMessage; - for (auto const& it : edgeCollectionInfos) { + for (auto const& it : edgeCollectionNames) { try { - opts.addEdgeFilter(isolate, edgeExample, it->getName(), errorMessage); + opts.addEdgeFilter(isolate, edgeExample, it, errorMessage); } catch (Exception& e) { // ELEMENT not found is expected, if there is no shape of this type in // this collection