8000 try to work around some assertions (#3296) · arangodb/arangodb@4ba38bf · GitHub
[go: up one dir, main page]
Skip to content

Commit 4ba38bf

Browse files
jsteemannfceller
jsteemann
authored and
fceller
committed
try to work around some assertions (#3296)
* remove obsolete values from relative config * warn about using obsolete config parameters
1 parent 8badead commit 4ba38bf

File tree

16 files changed

+146
-56
lines changed
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ SET(ARANGOD_SOURCES
356356
Transaction/Methods.cpp
357357
Transaction/Options.cpp
358358
Transaction/StandaloneContext.cpp
359+
Transaction/Status.cpp
359360
Transaction/V8Context.cpp
360361
Utils/Authentication.cpp
361362
Utils/CollectionKeys.cpp
Original file line numberDiff line numberDiff line change
@@ -1707,7 +1707,12 @@ int fetchEdgesFromEngines(
17071707
resSlice, "readIndex", 0);
17081708
VPackSlice edges = resSlice.get("edges");
17091709
for (auto const& e : VPackArrayIterator(edges)) {
1710-
VPackSlice id = transaction::helpers::extractIdFromDocument(e);
1710+
VPackSlice id = e.get(StaticStrings::IdString);
1711+
if (!id.isString()) {
1712+
// invalid id type
1713+
LOG_TOPIC(ERR, Logger::FIXME) << "got invalid edge id type: " << id.typeName();
1714+
continue;
1715+
}
17111716
StringRef idRef(id);
17121717
auto resE = cache.find(idRef);
17131718
if (resE == cache.end()) {
@@ -1809,7 +1814,13 @@ void fetchVerticesFromEngines(
18091814
}
18101815
TRI_ASSERT(result.find(key) == result.end());
18111816
auto val = VPackBuilder::clone(pair.value);
1812-
VPackSlice id = transaction::helpers::extractIdFromDocument(val.slice());
1817+
1818+
VPackSlice id = val.slice().get(StaticStrings::IdString);
1819+
if (!id.isString()) {
1820+
// invalid id type
1821+
LOG_TOPIC(ERR, Logger::FIXME) << "got invalid edge id type: " << id.typeName();
1822+
continue;
1823+
}
18131824
TRI_ASSERT(id.isString());
18141825
result.emplace(StringRef(id), val.steal());
18151826
}
@@ -2689,7 +2700,12 @@ int fetchEdgesFromEngines(
26892700
resSlice, "readIndex", 0);
26902701
VPackSlice edges = resSlice.get("edges");
26912702
for (auto const& e : VPackArrayIterator(edges)) {
2692-
VPackSlice id = transaction::helpers::extractIdFromDocument(e);
2703+
VPackSlice id = e.get(StaticStrings::IdString);
2704+
if (!id.isString()) {
2705+
// invalid id type
2706+
LOG_TOPIC(ERR, Logger::FIXME) << "got invalid edge id type: " << id.typeName();
2707+
continue;
2708+
}
26932709
StringRef idRef(id);
26942710
auto resE = cache.find(idRef);
26952711
if (resE == cache.end()) {
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ void BaseEngine::getVertexData(VPackSlice vertex, VPackBuilder& builder) {
209209
// Maybe handle differently
210210
}
211211

212-
StringRef vertex = id.substr(pos+1);
212+
StringRef vertex = id.substr(pos + 1);
213213
for (std::string const& shard : shards->second) {
214214
Result res = _trx->documentFastPathLocal(shard, vertex, mmdr, false);
215215
if (res.ok()) {
@@ -262,6 +262,9 @@ void BaseTraverserEngine::getEdges(VPackSlice vertex, size_t depth,
262262
if (edge.isString()) {
263263
edge = _opts->cache()->lookupToken(eid);
264264
}
265+
if (edge.isNull()) {
266+
return;
267+
}
265268
if (_opts->evaluateEdgeExpression(edge, StringRef(v), depth,
266269
cursorId)) {
267270
builder.add(edge);
@@ -277,6 +280,9 @@ void BaseTraverserEngine::getEdges(VPackSlice vertex, size_t depth,
277280
if (edge.isString()) {
278281
edge = _opts->cache()->lookupToken(eid);
279282
}
283+
if (edge.isNull()) {
284+
return;
285+
}
280286
if (_opts->evaluateEdgeExpression(edge, StringRef(vertex), depth,
281287
cursorId)) {
282288
builder.add(edge);
@@ -307,6 +313,9 @@ void BaseTraverserEngine::getVertexData(VPackSlice vertex, size_t depth,
307313
builder.add(VPackValue("vertices"));
308314

309315
auto workOnOneDocument = [&](VPackSlice v) {
316+
if (v.isNull()) {
317+
return;
318+
}
310319
StringRef id(v);
311320
size_t pos = id.find('/');
312321
if (pos == std::string::npos || pos + 1 == id.size()) {
@@ -325,7 +334,7 @@ void BaseTraverserEngine::getVertexData(VPackSlice vertex, size_t depth,
325334
// Maybe handle differently
326335
}
327336

328-
StringRef vertex = id.substr(pos+1);
337+
StringRef vertex = id.substr(pos + 1);
329338
for (std::string const& shard : shards->second) {
330339
Result res = _trx->documentFastPathLocal(shard, vertex, mmdr, false);
331340
if (res.ok()) {
@@ -402,6 +411,9 @@ void ShortestPathEngine::getEdges(VPackSlice vertex, bool backward,
402411
builder.openArray();
403412
if (vertex.isArray()) {
404413
for (VPackSlice v : VPackArrayIterator(vertex)) {
414+
if (!vertex.isString()) {
415+
continue;
416+
}
405417
TRI_ASSERT(v.isString());
406418
// result.clear();
407419
StringRef vertexId(v);
@@ -415,6 +427,9 @@ void ShortestPathEngine::getEdges(VPackSlice vertex, bool backward,
415427
VPackSlice edge, size_t cursorId) {
416428
if (edge.isString()) {
417429
edge = _opts->cache()->lookupToken(eid);
430+
}
431+
if (edge.isNull()) {
432+
return;
418433
}
419434
builder.add(edge);
420435
});
@@ -432,6 +447,9 @@ void ShortestPathEngine::getEdges(VPackSlice vertex, bool backward,
432447
if (edge.isString()) {
433448
edge = _opts->cache()->lookupToken(eid);
434449
}
450+
if (edge.isNull()) {
451+
return;
452+
}
435453
builder.add(edge);
436454
});
437455
// Result now contains all valid edges, probably multiples.
Original file line numberDiff line numberDiff line change
@@ -50,24 +50,21 @@ aql::AqlValue ClusterTraverserCache::fetchEdgeAqlResult(EdgeDocumentToken const&
5050
TRI_ASSERT(ServerState::instance()->isCoordinator());
5151
// FIXME: the ClusterTraverserCache lifetime is shorter then the query lifetime
5252
// therefore we cannot get away here without copying the result
53-
//return aql::AqlValue(aql::AqlValueHintDocumentNoCopy(token.vpack()));
5453
return aql::AqlValue(VPackSlice(token.vpack())); // will copy slice
5554
}
5655

57-
5856
aql::AqlValue ClusterTraverserCache::fetchVertexAqlResult(StringRef id) {
5957
// FIXME: this is only used for ShortestPath, where the shortestpath stuff
6058
// uses _edges to store its vertices
6159
TRI_ASSERT(ServerState::instance()->isCoordinator());
6260
auto it = _cache.find(id);
6361
if (it == _cache.end()) {
64-
LOG_TOPIC(ERR, Logger::GRAPHS) << __FUNCTION__ << " vertex not found";
62+
LOG_TOPIC(ERR, Logger::GRAPHS) << __FUNCTION__ << " vertex '" << id.toString() << "' not found";
6563
// Document not found return NULL
6664
return aql::AqlValue(aql::AqlValueHintNull());
6765
}
6866
// FIXME: the ClusterTraverserCache lifetime is shorter then the query lifetime
6967
// therefore we cannot get away here without copying the result
70-
//return aql::AqlValue(aql::AqlValueHintDocumentNoCopy(it->second.begin()));
7168
return aql::AqlValue(it->second); // will copy slice
7269
}
7370

@@ -81,7 +78,7 @@ void ClusterTraverserCache::insertVertexIntoResult(StringRef id,
8178
VPackBuilder& result) {
8279
auto it = _cache.find(id);
8380
if (it == _cache.end()) {
84-
LOG_TOPIC(ERR, Logger::GRAPHS) << __FUNCTION__ << " vertex not found";
81+
LOG_TOPIC(ERR, Logger::GRAPHS) << __FUNCTION__ << " vertex '" << id.toString() << "' not found";
8582
// Document not found append NULL
8683
result.add(VelocyPackHelper::NullValue());
8784
} else {
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,23 @@ TraverserCache::~TraverserCache() {}
5353
VPackSlice TraverserCache::lookupToken(EdgeDocumentToken const& idToken) {
5454
TRI_ASSERT(!ServerState::instance()->isCoordinator());
5555
auto col = _trx->vocbase()->lookupCollection(idToken.cid());
56-
TRI_ASSERT(col != nullptr);
56+
57+
if (col == nullptr) {
58+
// collection gone... should not happen
59+
LOG_TOPIC(ERR, arangodb::Logger::GRAPHS) << "Could not extract indexed edge document. collection not found";
60+
TRI_ASSERT(col != nullptr); // for maintainer mode
61+
return basics::VelocyPackHelper::NullValue();
62+
}
63+
5764
if (!col->readDocument(_trx, idToken.token(), *_mmdr.get())) {
58-
TRI_ASSERT(false);
5965
// We already had this token, inconsistent state. Return NULL in Production
60-
LOG_TOPIC(ERR, arangodb::Logger::GRAPHS) << "Could not extract indexed Edge Document, return 'null' instead. "
61-
<< "This is most likely a caching issue. Try: '"
62-
<< col->name() <<".unload(); " << col->name() << ".load()' in arangosh to fix this.";
66+
LOG_TOPIC(ERR, arangodb::Logger::GRAPHS) << "Could not extract indexed edge document, return 'null' instead. "
67+
<< "This is most likely a caching issue. Try: 'db."
68+
<< col->name() <<".unload(); db." << col->name() << ".load()' in arangosh to fix this.";
69+
TRI_ASSERT(false); // for maintainer mode
6370
return basics::VelocyPackHelper::NullValue();
6471
}
72+
6573
return VPackSlice(_mmdr->vpack());
6674
}
6775

@@ -71,9 +79,10 @@ VPackSlice TraverserCache::lookupInCollection(StringRef id) {
7179
if (pos == std::string::npos || pos + 1 == id.size()) {
7280
// Invalid input. If we get here somehow we managed to store invalid _from/_to
7381
// values or the traverser did a let an illegal start through
74-
TRI_ASSERT(false);
82+
TRI_ASSERT(false); // for maintainer mode
7583
return basics::VelocyPackHelper::NullValue();
7684
}
85+
7786
Result res = _trx->documentFastPathLocal(id.substr(0, pos).toString(),
7887
id.substr(pos + 1), *_mmdr, true);
7988
if (res.ok()) {
Original file line numberDiff line numberDiff line change
@@ -182,34 +182,32 @@ void InternalRestTraverserHandler::queryEngine() {
182182
}
183183

184184
switch (engine->getType()) {
185-
case BaseEngine::EngineType::TRAVERSER:
186-
{
187-
VPackSlice depthSlice = body.get("depth");
188-
if (!depthSlice.isInteger()) {
189-
generateError(ResponseCode::BAD, TRI_ERROR_HTTP_BAD_PARAMETER,
190-
"expecting 'depth' to be an integer value");
191-
return;
192-
}
193-
// Save Cast BaseTraverserEngines are all of type TRAVERSER
194-
auto eng = static_cast<BaseTraverserEngine*>(engine);
195-
TRI_ASSERT(eng != nullptr);
196-
eng->getEdges(keysSlice, depthSlice.getNumericValue<size_t>(), result);
197-
break;
185+
case BaseEngine::EngineType::TRAVERSER: {
186+
VPackSlice depthSlice = body.get("depth");
187+
if (!depthSlice.isInteger()) {
188+
generateError(ResponseCode::BAD, TRI_ERROR_HTTP_BAD_PARAMETER,
189+
"expecting 'depth' to be an integer value");
190+
return;
198191
}
199-
case BaseEngine::EngineType::SHORTESTPATH:
200-
{
201-
VPackSlice bwSlice = body.get("backward");
202-
if (!bwSlice.isBool()) {
203-
generateError(ResponseCode::BAD, TRI_ERROR_HTTP_BAD_PARAMETER,
204-
"expecting 'backward' to be a boolean value");
205-
return;
206-
}
207-
// Save Cast ShortestPathEngines are all of type SHORTESTPATH
208-
auto eng = static_cast<ShortestPathEngine*>(engine);
209-
TRI_ASSERT(eng != nullptr);
210-
eng->getEdges(keysSlice, bwSlice.getBoolean(), result);
211-
break;
192+
// Save Cast BaseTraverserEngines are all of type TRAVERSER
193+
auto eng = static_cast<BaseTraverserEngine*>(engine);
194+
TRI_ASSERT(eng != nullptr);
195+
eng->getEdges(keysSlice, depthSlice.getNumericValue<size_t>(), result);
196+
break;
197+
}
198+
case BaseEngine::EngineType::SHORTESTPATH: {
199+
VPackSlice bwSlice = body.get("backward");
200+
if (!bwSlice.isBool()) {
201+
generateError(ResponseCode::BAD, TRI_ERROR_HTTP_BAD_PARAMETER,
202+
"expecting 'backward' to be a boolean value");
203+
return;
212204
}
205+
// Save Cast ShortestPathEngines are all of type SHORTESTPATH
206+
auto eng = static_cast<ShortestPathEngine*>(engine);
207+
TRI_ASSERT(eng != nullptr);
208+
eng->getEdges(keysSlice, bwSlice.getBoolean(), result);
209+
break;
210+
}
213211
default:
214212
generateError(ResponseCode::BAD, TRI_ERROR_HTTP_BAD_PARAMETER,
215213
"this engine does not support the requested operation.");
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,9 @@ Result RocksDBTransactionState::commitTransaction(
293293
abortTransaction(activeTrx);
294294
}
295295
}
296-
updateStatus(transaction::Status::COMMITTED);
296+
if (res.ok()) {
297+
updateStatus(transaction::Status::COMMITTED);
298+
}
297299
}
298300

299301
unuseCollections(_nestingLevel);
Original file line numberDiff line numberDiff line change
342342

343343
/// @brief update the status of a transaction
344344
void TransactionState::updateStatus(transaction::Status status) {
345+
#ifdef ARANGODB_ENABLE_MAINTAINER_MODE
346+
if (_status != transaction::Status::CREATED &&
347+
_status != transaction::Status::RUNNING) {
348+
LOG_TOPIC(ERR, Logger::FIXME) << "trying to update transaction status with an invalid state. current: " << _status << ", future: " << status;
349+
}
350+
#endif
351+
345352
TRI_ASSERT(_status == transaction::Status::CREATED ||
346353
_status == transaction::Status::RUNNING);
347354

Original file line numberDiff line numberDiff line change
@@ -92,8 +92,8 @@ StringRef transaction::helpers::extractKeyPart(VPackSlice slice) {
9292
/// @brief extract the _id attribute from a slice, and convert it into a
9393
/// string, static method
9494
std::string transaction::helpers::extractIdString(CollectionNameResolver const* resolver,
95-
VPackSlice slice,
96-
VPackSlice const& base) {
95+
VPackSlice slice,
96+
VPackSlice const& base) {
9797
VPackSlice id;
9898

9999
if (slice.isExternal()) {
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ namespace helpers {
6363
/// the document must have at least two attributes, and _id is supposed to
6464
/// be the second one
6565
/// note that this may return a Slice of type Custom!
66+
/// do NOT call this method when
67+
/// - the input slice is not a database document
68+
/// - you are not willing to deal with slices of type Custom
6669
VPackSlice extractIdFromDocument(VPackSlice);
6770

6871
/// @brief quick access to the _from attribute in a database document