8000 Bug fix/simplify things (#6516) · arangodb/arangodb@c380515 · GitHub
[go: up one dir, main page]

Skip to content

Commit c380515

Browse files
authored
Bug fix/simplify things (#6516)
1 parent 78fc6aa commit c380515

20 files changed

+338
-551
lines changed

arangod/Cluster/ClusterComm.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ size_t ClusterComm::performRequests(std::vector<ClusterCommRequest>& requests,
839839
ClusterCommTimeout timeout, size_t& nrDone,
840840
arangodb::LogTopic const& logTopic,
841841
bool retryOnCollNotFound) {
842-
if (requests.size() == 0) {
842+
if (requests.empty()) {
843843
nrDone = 0;
844844
return 0;
845845
}

arangod/ClusterEngine/ClusterCollection.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
#include "Utils/OperationOptions.h"
4444
#include "VocBase/LocalDocumentId.h"
4545
#include "VocBase/LogicalCollection.h"
46+
#include "VocBase/ManagedDocumentResult.h"
4647
#include "VocBase/ticks.h"
4748
#include "VocBase/voc-types.h"
4849

arangod/ClusterEngine/ClusterCollection.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
#include "ClusterEngine/Common.h"
3131
#include "StorageEngine/PhysicalCollection.h"
3232
#include "VocBase/LogicalCollection.h"
33-
#include "VocBase/ManagedDocumentResult.h"
3433

3534
namespace rocksdb {
3635
class Transaction;

arangod/MMFiles/MMFilesCollection.cpp

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,55 @@
6565
#include "VocBase/KeyGenerator.h"
6666
#include "VocBase/LocalDocumentId.h"
6767
#include "VocBase/LogicalCollection.h"
68+
#include "VocBase/ManagedDocumentResult.h"
6869
#include "VocBase/ticks.h"
6970

7071
using namespace arangodb;
7172
using Helper = arangodb::basics::VelocyPackHelper;
7273

74+
/// @brief state during opening of a collection
75+
namespace arangodb {
76+
struct OpenIteratorState {
77+
LogicalCollection* _collection;
78+
arangodb::MMFilesPrimaryIndex* _primaryIndex;
79+
TRI_voc_tid_t _tid;
80+
TRI_voc_fid_t _fid;
81+
std::unordered_map<TRI_voc_fid_t, MMFilesDatafileStatisticsContainer*>
82+
_stats;
83+
MMFilesDatafileStatisticsContainer* _dfi;
84+
transaction::Methods* _trx;
85+
ManagedDocumentResult _mdr;
86+
IndexLookupContext _context;
87+
uint64_t _deletions;
88+
uint64_t _documents;
89+
int64_t _initialCount;
90+
91+
OpenIteratorState(LogicalCollection* collection, transaction::Methods* trx)
92+
: _collection(collection),
93+
_primaryIndex(
94+
static_cast<MMFilesCollection*>(collection->getPhysical())
95+
->primaryIndex()),
96+
_tid(0),
97+
_fid(0),
98+
_stats(),
99+
_dfi(nullptr),
100+
_trx(trx),
101+
_context(trx, collection, &_mdr, 1),
102+
_deletions(0),
103+
_documents(0),
104+
_initialCount(-1) {
105+
TRI_ASSERT(collection != nullptr);
106+
TRI_ASSERT(trx != nullptr);
107+
}
108+
109+
~OpenIteratorState() {
110+
for (auto& it : _stats) {
111+
delete it.second;
112+
}
113+
}
114+
};
115+
}
116+
73117
namespace {
74118

75119
/// @brief helper class for filling indexes
@@ -100,7 +144,7 @@ class MMFilesIndexFillerTask : public basics::LocalTask {
100144

101145
/// @brief find a statistics container for a given file id
102146
static MMFilesDatafileStatisticsContainer* FindDatafileStats(
103-
MMFilesCollection::OpenIteratorState* state, TRI_voc_fid_t fid) {
147+
OpenIteratorState* state, TRI_voc_fid_t fid) {
104148
auto it = state->_stats.find(fid);
105149

106150
if (it != state->_stats.end()) {
@@ -216,7 +260,7 @@ PhysicalCollection* MMFilesCollection::clone(LogicalCollection& logical) const {
216260
/// @brief process a document (or edge) marker when opening a collection
217261
int MMFilesCollection::OpenIteratorHandleDocumentMarker(
218262
MMFilesMarker const* marker, MMFilesDatafile* datafile,
219-
MMFilesCollection::OpenIteratorState* state) {
263+
OpenIteratorState* state) {
220264
LogicalCollection* collection = state->_collection;
221265
TRI_ASSERT(collection != nullptr);
222266
auto physical = static_cast<MMFilesCollection*>(collection->getPhysical());
@@ -265,7 +309,7 @@ int MMFilesCollection::OpenIteratorHandleDocumentMarker(
265309
// no primary index lock required here because we are the only ones reading
266310
// from the index ATM
267311
MMFilesSimpleIndexElement* found =
268-
state->_primaryIndex->lookupKeyRef(trx, keySlice, state->_mmdr);
312+
state->_primaryIndex->lookupKeyRef(trx, keySlice, state->_mdr);
269313

270314
// it is a new entry
271315
if (found == nullptr || !found->isSet()) {
@@ -274,7 +318,7 @@ int MMFilesCollection::OpenIteratorHandleDocumentMarker(
274318
// insert into primary index
275319
Result res = state->_primaryIndex->insertKey(trx, localDocumentId,
276320
VPackSlice(vpack),
277-
state->_mmdr,
321+
state->_mdr,
278322
Index::OperationMode::normal);
279323

280324
if (res.fail()) {
@@ -336,7 +380,7 @@ int MMFilesCollection::OpenIteratorHandleDocumentMarker(
336380
/// @brief process a deletion marker when opening a collection
337381
int MMFilesCollection::OpenIteratorHandleDeletionMarker(
338382
MMFilesMarker const* marker, MMFilesDatafile* datafile,
339-
MMFilesCollection::OpenIteratorState* state) {
383+
OpenIteratorState* state) {
340384
LogicalCollection* collection = state->_collection;
341385
TRI_ASSERT(collection != nullptr);
342386
auto physical = static_cast<MMFilesCollection*>(collection->getPhysical());
@@ -371,7 +415,7 @@ int MMFilesCollection::OpenIteratorHandleDeletionMarker(
371415
// no primary index lock required here because we are the only ones reading
372416
// from the index ATM
373417
MMFilesSimpleIndexElement found =
374-
state->_primaryIndex->lookupKey(trx, keySlice, state->_mmdr);
418+
state->_primaryIndex->lookupKey(trx, keySlice, state->_mdr);
375419

376420
// it is a new entry, so we missed the create
377421
if (!found) {
@@ -407,7 +451,7 @@ int MMFilesCollection::OpenIteratorHandleDeletionMarker(
407451
state->_dfi->numberDeletions++;
408452

409453
state->_primaryIndex->removeKey(trx, oldLocalDocumentId, VPackSlice(vpack),
410-
state->_mmdr, Index::OperationMode::normal);
454+
state->_mdr, Index::OperationMode::normal);
411455

412456
physical->removeLocalDocumentId(oldLocalDocumentId, true);
413457
}
@@ -417,7 +461,7 @@ int MMFilesCollection::OpenIteratorHandleDeletionMarker(
417461

418462
/// @brief iterator for open
419463
bool MMFilesCollection::OpenIterator(MMFilesMarker const* marker,
420-
MMFilesCollection::OpenIteratorState* data,
464+
OpenIteratorState* data,
421465
MMFilesDatafile* datafile) {
422466
TRI_voc_tick_t const tick = marker->getTick();
423467
MMFilesMarkerType const type = marker->getType();
@@ -434,15 +478,8 @@ bool MMFilesCollection::OpenIterator(MMFilesMarker const* marker,
434478
if (tick > datafile->_dataMax) {
435479
datafile->_dataMax = tick;
436480
}
437-
438-
if (++data->_operations % 1024 == 0) {
439-
data->_mmdr.reset();
440-
}
441481
} else if (type == TRI_DF_MARKER_VPACK_REMOVE) {
442482
res = OpenIteratorHandleDeletionMarker(marker, datafile, data);
443-
if (++data->_operations % 1024 == 0) {
444-
data->_mmdr.reset();
445-
}
446483
} else {
447484
if (type == TRI_DF_MARKER_HEADER) {
448485
// ensure there is a datafile info entry for each datafile of the
@@ -3290,7 +3327,7 @@ Result MMFilesCollection::update(
32903327

32913328
if (newSlice.length() <= 1) {
32923329
// no need to do anything
3293-
result = std::move(previous);
3330+
result = previous;
32943331

32953332
if (_logicalCollection.waitForSync()) {
32963333
options.waitForSync = true;

arangod/MMFiles/MMFilesCollection.h

Lines changed: 2 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,17 @@
3838
#include "VocBase/KeyGenerator.h"
3939
#include "VocBase/LocalDocumentId.h"
4040
#include "VocBase/LogicalCollection.h"
41-
#include "VocBase/ManagedDocumentResult.h"
4241

4342
struct MMFilesDatafile;
4443
struct MMFilesMarker;
4544

4645
namespace arangodb {
47-
48-
4946
class LogicalCollection;
5047
class ManagedDocumentResult;
5148
struct MMFilesDocumentOperation;
5249
class MMFilesPrimaryIndex;
5350
class MMFilesWalMarker;
51+
struct OpenIteratorState;
5452
class Result;
5553
class TransactionState;
5654

@@ -72,51 +70,7 @@ class MMFilesCollection final : public PhysicalCollection {
7270
TRI_ASSERT(phys != nullptr);
7371
return toMMFilesCollection(phys);
7472
}
75-
76-
/// @brief state during opening of a collection
77-
struct OpenIteratorState {
78-
LogicalCollection* _collection;
79-
arangodb::MMFilesPrimaryIndex* _primaryIndex;
80-
TRI_voc_tid_t _tid;
81-
TRI_voc_fid_t _fid;
82-
std::unordered_map<TRI_voc_fid_t, MMFilesDatafileStatisticsContainer*>
83-
_stats;
84-
MMFilesDatafileStatisticsContainer* _dfi;
85-
transaction::Methods* _trx;
86-
ManagedDocumentResult _mmdr;
87-
IndexLookupContext _context;
88-
uint64_t _deletions;
89-
uint64_t _documents;
90-
uint64_t _operations;
91-
int64_t _initialCount;
92-
93-
OpenIteratorState(LogicalCollection* collection, transaction::Methods* trx)
94-
: _collection(collection),
95-
_primaryIndex(
96-
static_cast<MMFilesCollection*>(collection->getPhysical())
97-
->primaryIndex()),
98-
_tid(0),
99-
_fid(0),
100-
_stats(),
101-
_dfi(nullptr),
102-
_trx(trx),
103-
_mmdr(),
104-
_context(trx, collection, &_mmdr, 1),
105-
_deletions(0),
106-
_documents(0),
107-
_operations(0),
108-
_initialCount(-1) {
109-
TRI_ASSERT(collection != nullptr);
110-
TRI_ASSERT(trx != nullptr);
111-
}
112-
113-
~OpenIteratorState() {
114-
for (auto& it : _stats) {
115-
delete it.second;
116-
}
117-
}
118-
};
119-
73+
12074
struct DatafileDescription {
12175
MMFilesDatafile const* _data;
12276
TRI_voc_tick_t _dataMin;

arangod/MMFiles/MMFilesCollectionKeys.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "Utils/SingleCollectionTransaction.h"
3636
#include "Transaction/StandaloneContext.h"
3737
#include "VocBase/LogicalCollection.h"
38+
#include "VocBase/ManagedDocumentResult.h"
3839
#include "VocBase/vocbase.h"
3940

4041
#include <velocypack/Builder.h>
@@ -110,13 +111,13 @@ void MMFilesCollectionKeys::create(TRI_voc_tick_t maxTick) {
110111
THROW_ARANGO_EXCEPTION(res);
111112
}
112113

113-
ManagedDocumentResult mmdr;
114-
MMFilesCollection *mmColl = MMFilesCollection::toMMFilesCollection(_collection);
114+
ManagedDocumentResult mdr;
115+
MMFilesCollection* mmColl = MMFilesCollection::toMMFilesCollection(_collection);
115116

116117
trx.invokeOnAllElements(
117-
_collection->name(), [this, &trx, &maxTick, &mmdr, &mmColl](LocalDocumentId const& token) {
118-
if (mmColl->readDocumentConditional(&trx, token, maxTick, mmdr)) {
119-
_vpack.emplace_back(mmdr.vpack());
118+
_collection->name(), [this, &trx, &maxTick, &mdr, &mmColl](LocalDocumentId const& token) {
119+
if (mmColl->readDocumentConditional(&trx, token, maxTick, mdr)) {
120+
_vpack.emplace_back(mdr.vpack());
120121
}
121122
return true;
122123
});

arangod/MMFiles/MMFilesEdgeIndex.cpp

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "Transaction/Methods.h"
4141
#include "Utils/CollectionNameResolver.h"
4242
#include "VocBase/LogicalCollection.h"
43+
#include "VocBase/ManagedDocumentResult.h"
4344

4445
#include <velocypack/Iterator.h>
4546
#include <velocypack/velocypack-aliases.h>
@@ -55,12 +56,12 @@ static std::vector<std::vector<arangodb::basics::AttributeName>> const
5556

5657
MMFilesEdgeIndexIterator::MMFilesEdgeIndexIterator(
5758
LogicalCollection* collection, transaction::Methods* trx,
58-
ManagedDocumentResult* mmdr, arangodb::MMFilesEdgeIndex const* index,
59+
ManagedDocumentResult* mdr, arangodb::MMFilesEdgeIndex const* index,
5960
TRI_MMFilesEdgeIndexHash_t const* indexImpl,
6061
std::unique_ptr<VPackBuilder> keys)
6162
: IndexIterator(collection, trx),
6263
_index(indexImpl),
63-
_context(trx, collection, mmdr, index->fields().size()),
64+
_context(trx, collection, mdr, index->fields().size()),
6465
_keys(std::move(keys)),
6566
_iterator(_keys->slice()),
6667
_posInBuffer(0),
@@ -419,7 +420,7 @@ bool MMFilesEdgeIndex::supportsFilterCondition(
419420

420421
/// @brief creates an IndexIterator for the given Condition
421422
IndexIterator* MMFilesEdgeIndex::iteratorForCondition(
422-
transaction::Methods* trx, ManagedDocumentResult* mmdr,
423+
transaction::Methods* trx, ManagedDocumentResult* mdr,
423424
arangodb::aql::AstNode const* node,
424425
arangodb::aql::Variable const* reference,
425426
IndexIteratorOptions const& opts) {
@@ -442,7 +443,7 @@ IndexIterator* MMFilesEdgeIndex::iteratorForCondition(
442443

443444
if (comp->type == aql::NODE_TYPE_OPERATOR_BINARY_EQ) {
444445
// a.b == value
445-
return createEqIterator(trx, mmdr, attrNode, valNode);
446+
return createEqIterator(trx, mdr, attrNode, valNode);
446447
}
447448

448449
if (comp->type == aql::NODE_TYPE_OPERATOR_BINARY_IN) {
@@ -452,7 +453,7 @@ IndexIterator* MMFilesEdgeIndex::iteratorForCondition(
452453
return new EmptyIndexIterator(&_collection, trx);
453454
}
454455

455-
return createInIterator(trx, mmdr, attrNode, valNode);
456+
return createInIterator(trx, mdr, attrNode, valNode);
456457
}
457458

458459
// operator type unsupported
@@ -469,7 +470,7 @@ arangodb::aql::AstNode* MMFilesEdgeIndex::specializeCondition(
469470

470471
/// @brief create the iterator
471472
IndexIterator* MMFilesEdgeIndex::createEqIterator(
472-
transaction::Methods* trx, ManagedDocumentResult* mmdr,
473+
transaction::Methods* trx, ManagedDocumentResult* mdr,
473474
arangodb::aql::AstNode const* attrNode,
474475
arangodb::aql::AstNode const* valNode) const {
475476
// lease builder, but immediately pass it to the unique_ptr so we don't leak
@@ -489,7 +490,7 @@ IndexIterator* MMFilesEdgeIndex::createEqIterator(
489490
return new MMFilesEdgeIndexIterator(
490491
&_collection,
491492
trx,
492-
mmdr,
493+
mdr,
493494
this,
494495
isFrom ? _edgesFrom.get() : _edgesTo.get(),
495496
std::move(keys)
@@ -498,7 +499,7 @@ IndexIterator* MMFilesEdgeIndex::createEqIterator(
498499

499500
/// @brief create the iterator
500501
IndexIterator* MMFilesEdgeIndex::createInIterator(
501-
transaction::Methods* trx, ManagedDocumentResult* mmdr,
502+
transaction::Methods* trx, ManagedDocumentResult* mdr,
502503
arangodb::aql::AstNode const* attrNode,
503504
arangodb::aql::AstNode const* valNode) const {
504505
// lease builder, but immediately pass it to the unique_ptr so we don't leak
@@ -525,7 +526,7 @@ IndexIterator* MMFilesEdgeIndex::createInIterator(
525526
return new MMFilesEdgeIndexIterator(
526527
&_collection,
527528
trx,
528-
mmdr,
529+
mdr,
529530
this,
530531
isFrom ? _edgesFrom.get() : _edgesTo.get(),
531532
std::move(keys)

arangod/MMFiles/MMFilesHashIndex.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include "Transaction/Context.h"
3939
#include "Transaction/Helpers.h"
4040
#include "VocBase/LogicalCollection.h"
41+
#include "VocBase/ManagedDocumentResult.h"
4142

4243
#include <velocypack/Iterator.h>
4344
#include <velocypack/velocypack-aliases.h>

0 commit comments

Comments
 (0)
0