8000 Bug fix/dont use indexes in progress (#10432) · arangodb/arangodb@2c4f7bb · GitHub
[go: up one dir, main page]
Skip to content

Commit 2c4f7bb

Browse files
jsteemanngnusi
jsteemann
authored and
gnusi
committed
Bug fix/dont use indexes in progress (#10432)
* don't return any in-progress indexes * fix handling of in-progress indexes * add test * address review comments
1 parent aff5e71 commit 2c4f7bb

File tree

12 files changed

+180
-30
lines changed

12 files changed

+180
-30
lines changed

‎arangod/IResearch/IResearchView.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,11 +360,12 @@ arangodb::Result IResearchView::appendVelocyPackImpl( // append JSON
360360
static const std::function<bool(irs::string_ref const& key)> persistenceAcceptor =
361361
[](irs::string_ref const&) -> bool { return true; };
362362

363-
auto& acceptor = context == Serialization::Persistence || context == Serialization::Inventory
363+
auto& acceptor =
364+
(context == Serialization::Persistence || context == Serialization::PersistenceWithInProgress || context == Serialization::Inventory)
364365
? persistenceAcceptor
365366
: propertiesAcceptor;
366367

367-
if (context == Serialization::Persistence) {
368+
if (context == Serialization::Persistence || context == Serialization::PersistenceWithInProgress) {
368369
if (arangodb::ServerState::instance()->isSingleServer()) {
369370
auto res = arangodb::LogicalViewHelperStorageEngine::properties(builder, *this);
370371

@@ -401,7 +402,7 @@ arangodb::Result IResearchView::appendVelocyPackImpl( // append JSON
401402
return {};
402403
}
403404

404-
if (context == Serialization::Persistence) {
405+
if (context == Serialization::Persistence || context == Serialization::PersistenceWithInProgress) {
405406
IResearchViewMetaState metaState;
406407

407408
for (auto& entry : _links) {

‎arangod/IResearch/IResearchViewCoordinator.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,10 @@ Result IResearchViewCoordinator::appendVelocyPackImpl(
188188

189189
auto* acceptor = &propertiesAcceptor;
190190

191-
if (context == Serialization::Persistence || context == Serialization::Inventory) {
192-
auto res = LogicalViewHelperClusterInfo::properties(builder, *this);
191+
if (context == Serialization::Persistence ||
192+
context == Serialization::PersistenceWithInProgress ||
193+
context == Serialization::Inventory) {
194+
auto res = arangodb::LogicalViewHelperClusterInfo::properties(builder, *this);
193195

194196
if (!res.ok()) {
195197
return res;

‎arangod/MMFiles/MMFilesCollection.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ arangodb::Result MMFilesCollection::persistProperties() {
295295
try {
296296
auto infoBuilder = _logicalCollection.toVelocyPackIgnore(
297297
{"path", "statusString"},
298-
LogicalDataSource::Serialization::Persistence);
298+
LogicalDataSource::Serialization::PersistenceWithInProgress);
299299
MMFilesCollectionMarker marker(TRI_DF_MARKER_VPACK_CHANGE_COLLECTION,
300300
_logicalCollection.vocbase().id(),
301301
_logicalCollection.id(), infoBuilder.slice());
@@ -2281,7 +2281,7 @@ std::shared_ptr<Index> MMFilesCollection::createIndex(transaction::Methods& trx,
22812281
if (!engine->inRecovery()) {
22822282
auto builder = _logicalCollection.toVelocyPackIgnore(
22832283
{"path", "statusString"},
2284-
LogicalDataSource::Serialization::Persistence);
2284+
LogicalDataSource::Serialization::PersistenceWithInProgress);
22852285
_logicalCollection.properties(builder.slice(),
22862286
false); // always a full-update
22872287
}
@@ -2419,7 +2419,7 @@ bool MMFilesCollection::dropIndex(TRI_idx_iid_t iid) {
24192419
{
24202420
auto builder = _logicalCollection.toVelocyPackIgnore(
24212421
{"path", "statusString"},
2422-
LogicalDataSource::Serialization::Persistence);
2422+
LogicalDataSource::Serialization::PersistenceWithInProgress);
24232423

24242424
_logicalCollection.properties(builder.slice(),
24252425
false); // always a full-update

‎arangod/RestHandler/RestIndexHandler.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,9 @@ RestStatus RestIndexHandler::getSelectivityEstimates() {
217217
builder.add(StaticStrings::Code, VPackValue(static_cast<int>(rest::ResponseCode::OK)));
218218
builder.add("indexes", VPackValue(VPackValueType::Object));
219219
for (std::shared_ptr<Index> idx : idxs) {
220+
if (idx->inProgress() || idx->isHidden()) {
221+
continue;
222+
}
220223
std::string name = coll->name();
221224
name.push_back(TRI_INDEX_HANDLE_SEPARATOR_CHR);
222225
name.append(std::to_string(idx->id()));

‎arangod/RocksDBEngine/RocksDBCollection.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ std::shared_ptr<Index> RocksDBCollection::createIndex(VPackSlice const& info,
402402
if (!engine->inRecovery()) { // write new collection marker
403403
auto builder = _logicalCollection.toVelocyPackIgnore(
404404
{"path", "statusString"},
405-
LogicalDataSource::Serialization::Persistence);
405+
LogicalDataSource::Serialization::PersistenceWithInProgress);
406406
VPackBuilder indexInfo;
407407
idx->toVelocyPack(indexInfo, Index::makeFlags(Index::Serialize::Internals));
408408
res = engine->writeCreateCollectionMarker(_logicalCollection.vocbase().id(),
@@ -484,7 +484,7 @@ bool RocksDBCollection::dropIndex(TRI_idx_iid_t iid) {
484484
auto builder = // RocksDB path
485485
_logicalCollection.toVelocyPackIgnore(
486486
{"path", "statusString"},
487-
LogicalDataSource::Serialization::Persistence);
487+
LogicalDataSource::Serialization::PersistenceWithInProgress);
488488

489489
// log this event in the WAL and in the collection meta-data
490490
res = engine->writeCreateCollectionMarker( // write marker

‎arangod/RocksDBEngine/RocksDBEngine.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,7 +1248,7 @@ std::string RocksDBEngine::createCollection(TRI_vocbase_t& vocbase,
12481248

12491249
auto builder = collection.toVelocyPackIgnore(
12501250
{"path", "statusString"},
1251-
LogicalDataSource::Serialization::Persistence);
1251+
LogicalDataSource::Serialization::PersistenceWithInProgress);
12521252
TRI_UpdateTickServer(static_cast<TRI_voc_tick_t>(cid));
12531253

12541254
int res =
@@ -1401,7 +1401,7 @@ void RocksDBEngine::changeCollection(TRI_vocbase_t& vocbase,
14011401
LogicalCollection const& collection, bool doSync) {
14021402
auto builder = collection.toVelocyPackIgnore(
14031403
{"path", "statusString"},
1404-
LogicalDataSource::Serialization::Persistence);
1404+
LogicalDataSource::Serialization::PersistenceWithInProgress);
14051405
int res =
14061406
writeCreateCollectionMarker(vocbase.id(), collection.id(), builder.slice(),
14071407
RocksDBLogValue::CollectionChange(vocbase.id(),
@@ -1417,7 +1417,7 @@ arangodb::Result RocksDBEngine::renameCollection(TRI_vocbase_t& vocbase,
14171417
std::string const& oldName) {
14181418
auto builder = collection.toVelocyPackIgnore(
14191419
{"path", "statusString"},
1420-
LogicalDataSource::Serialization::Persistence);
1420+
LogicalDataSource::Serialization::PersistenceWithInProgress);
14211421
int res = writeCreateCollectionMarker(
14221422
vocbase.id(), collection.id(), builder.slice(),
14231423
RocksDBLogValue::CollectionRename(vocbase.id(), collection.id(),
@@ -1445,7 +1445,7 @@ Result RocksDBEngine::createView(TRI_vocbase_t& vocbase, TRI_voc_cid_t id,
14451445
VPackBuilder props;
14461446

14471447
props.openObject();
1448-
view.properties(props, LogicalDataSource::Serialization::Persistence);
1448+
view.properties(props, LogicalDataSource::Serialization::PersistenceWithInProgress);
14491449
props.close();
14501450

14511451
RocksDBValue const value = RocksDBValue::View(props.slice());
@@ -1470,7 +1470,7 @@ arangodb::Result RocksDBEngine::dropView(TRI_vocbase_t const& vocbase,
14701470
VPackBuilder builder;
14711471

14721472
builder.openObject();
1473-
view.properties(builder, LogicalDataSource::Serialization::Persistence);
1473+
view.properties(builder, LogicalDataSource::Serialization::PersistenceWithInProgress);
14741474
builder.close();
14751475

14761476
auto logValue =
@@ -1516,7 +1516,7 @@ Result RocksDBEngine::changeView(TRI_vocbase_t& vocbase,
15161516
VPackBuilder infoBuilder;
15171517

15181518
infoBuilder.openObject();
1519-
view.properties(infoBuilder, LogicalDataSource::Serialization::Persistence);
1519+
view.properties(infoBuilder, LogicalDataSource::Serialization::PersistenceWithInProgress);
15201520
infoBuilder.close();
15211521

15221522
RocksDBLogValue log = RocksDBLogValue::ViewChange(vocbase.id(), view.id());

‎arangod/Transaction/Methods.cpp

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,8 @@ std::pair<bool, bool> transaction::Methods::findIndexHandleForAndNode(
555555
auto considerIndex = [&bestIndex, &bestCost, &bestSupportsFilter, &bestSupportsSort,
556556
&indexes, node, reference, itemsInCollection,
557557
&sortCondition](std::shared_ptr<Index> const& idx) -> void {
558+
TRI_ASSERT(!idx->inProgress());
559+
558560
double filterCost = 0.0;
559561
double sortCost = 0.0;
560562
size_t itemsInIndex = itemsInCollection;
@@ -2739,6 +2741,8 @@ bool transaction::Methods::getIndexForSortCondition(
27392741

27402742
auto considerIndex = [reference, sortCondition, itemsInIndex, &bestCost, &bestIndex,
27412743
&coveredAttributes](std::shared_ptr<Index> const& idx) -> void {
2744+
TRI_ASSERT(!idx->inProgress());
2745+
27422746
Index::SortCosts costs =
27432747
idx->supportsSortCondition(sortCondition, reference, itemsInIndex);
27442748
if (costs.supportsCondition &&
@@ -2813,6 +2817,7 @@ std::unique_ptr<IndexIterator> transaction::Methods::indexScanForCondition(
28132817
}
28142818

28152819
// Now create the Iterator
2820+
TRI_ASSERT(!idx->inProgress());
28162821
return idx->iteratorForCondition(this, condition, var, opts);
28172822
}
28182823

@@ -3003,7 +3008,7 @@ Result transaction::Methods::unlockRecursive(TRI_voc_cid_t cid, AccessMode::Type
30033008

30043009
/// @brief get list of indexes for a collection
30053010
std::vector<std::shared_ptr<Index>> transaction::Methods::indexesForCollection(
3006-
std::string const& collectionName, bool withHidden) {
3011+
std::string const& collectionName) {
30073012
if (_state->isCoordinator()) {
30083013
return indexesForCollectionCoordinator(collectionName);
30093014
}
@@ -3012,13 +3017,12 @@ std::vector<std::shared_ptr<Index>> transaction::Methods::indexesForCollection(
30123017
TRI_voc_cid_t cid = addCollectionAtRuntime(collectionName, AccessMode::Type::READ);
30133018
std::shared_ptr<LogicalCollection> const& document = trxCollection(cid)->collection();
30143019
std::vector<std::shared_ptr<Index>> indexes = document->getIndexes();
3015-
if (!withHidden) {
3016-
indexes.erase(std::remove_if(indexes.begin(), indexes.end(),
3017-
[](std::shared_ptr<Index> x) {
3018-
return x->isHidden();
3019-
}),
3020-
indexes.end());
3021-
}
3020+
3021+
indexes.erase(std::remove_if(indexes.begin(), indexes.end(),
3022+
[](std::shared_ptr<Index> const& x) {
3023+
return x->isHidden();
3024+
}),
3025+
indexes.end());
30223026
return indexes;
30233027
}
30243028

@@ -3049,7 +3053,14 @@ std::vector<std::shared_ptr<Index>> transaction::Methods::indexesForCollectionCo
30493053
collection->clusterIndexEstimates(true);
30503054
}
30513055

3052-
return collection->getIndexes();
3056+
std::vector<std::shared_ptr<Index>> indexes = collection->getIndexes();
3057+
3058+
indexes.erase(std::remove_if(indexes.begin(), indexes.end(),
3059+
[](std::shared_ptr<Index> const& x) {
3060+
return x->isHidden();
3061+
}),
3062+
indexes.end());
3063+
return indexes;
30533064
}
30543065

30553066
/// @brief get the index by it's identifier. Will either throw or

‎arangod/Transaction/Methods.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ class Methods {
414414

415415
/// @brief get all indexes for a collection name
416416
ENTERPRISE_VIRT std::vector<std::shared_ptr<arangodb::Index>> indexesForCollection(
417-
std::string const&, bool withHidden = false);
417+
std::string const& collectionName);
418418

419< 10000 code>419
/// @brief Lock all collections. Only works for selected sub-classes
420420
virtual int lockCollections();

‎arangod/VocBase/LogicalCollection.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,8 @@ void LogicalCollection::toVelocyPackForClusterInventory(VPackBuilder& result,
619619

620620
arangodb::Result LogicalCollection::appendVelocyPack(arangodb::velocypack::Builder& result,
621621
Serialization context) const {
622-
bool const forPersistence = (context == Serialization::Persistence);
622+
bool const forPersistence = (context == Serialization::Persistence || context == Serialization::PersistenceWithInProgress);
623+
bool const showInProgress = (context == Serialization::PersistenceWithInProgress);
623624

624625
// We write into an open object
625626
TRI_ASSERT(result.isOpenObject());
@@ -662,8 +663,8 @@ arangodb::Result LogicalCollection::appendVelocyPack(arangodb::velocypack::Build
662663
if (forPersistence) {
663664
indexFlags = Index::makeFlags(Index::Serialize::Internals);
664665
}
665-
auto filter = [indexFlags, forPersistence](arangodb::Index const* idx, decltype(Index::makeFlags())& flags) {
666-
if (forPersistence || (!idx->inProgress() && !idx->isHidden())) {
666+
auto filter = [indexFlags, forPersistence, showInProgress](arangodb::Index const* idx, decltype(Index::makeFlags())& flags) {
667+
if ((forPersistence || !idx->isHidden()) && (showInProgress || !idx->inProgress())) {
667668
flags = indexFlags;
668669
return true;
669670
}

‎arangod/VocBase/LogicalDataSource.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ Result LogicalDataSource::properties(velocypack::Builder& builder,
206206
// note: includeSystem and forPersistence are not 100% synonymous,
207207
// however, for our purposes this is an okay mapping; we only set
208208
// includeSystem if we are persisting the properties
209-
if (context == Serialization::Persistence) {
209+
if (context == Serialization::Persistence || context == Serialization::PersistenceWithInProgress) {
210210
builder.add(StaticStrings::DataSourceDeleted, velocypack::Value(deleted()));
211211
builder.add(StaticStrings::DataSourceSystem, velocypack::Value(system()));
212212

0 commit comments

Comments
 (0)
0