Skip to content

Commit 4f3f3a4

Browse files
lankasEvergreen Agent
authored andcommitted
SERVER-49846 Drop internal idents on startup if there are no index builds to resume
1 parent 2da98e0 commit 4f3f3a4

File tree

4 files changed

+28
-8
lines changed

4 files changed

+28
-8
lines changed

jstests/noPassthrough/libs/index_build.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,7 @@ const ResumableIndexBuildTest = class {
575575
// Ensure that the resumable index build failed as expected.
576576
if (failWhileParsing) {
577577
assert(RegExp("4916300.*").test(rawMongoProgramOutput()));
578+
assert(RegExp("22257.*").test(rawMongoProgramOutput()));
578579
} else {
579580
assert(RegExp("4841701.*" + buildUUID).test(rawMongoProgramOutput()));
580581
}
@@ -584,5 +585,14 @@ const ResumableIndexBuildTest = class {
584585

585586
ResumableIndexBuildTest.checkIndexes(
586587
rst, dbName, collName, indexName, postIndexBuildInserts);
588+
589+
// If we fail after parsing, any remaining internal idents will only be cleaned up after
590+
// another restart.
591+
if (!failWhileParsing) {
592+
clearRawMongoProgramOutput();
593+
rst.stop(primary);
594+
rst.start(primary, {noCleanData: true});
595+
assert(RegExp("22257.*").test(rawMongoProgramOutput()));
596+
}
587597
}
588598
};

jstests/noPassthrough/restart_index_build_if_resume_fails.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ ResumableIndexBuildTest.runFailToResume(rst,
3535
[{a: 2}, {a: 3}],
3636
[{a: 4}, {a: 5}],
3737
true /* failWhileParsing */);
38-
assert.commandWorked(
39-
primary.adminCommand({configureFailPoint: 'failToParseResumeIndexInfo', mode: 'off'}));
4038

4139
ResumableIndexBuildTest.runFailToResume(
4240
rst, dbName, collName, {a: 1}, "failSetUpResumeIndexBuild", [{a: 6}, {a: 7}], [{a: 8}, {a: 9}]);

src/mongo/db/storage/storage_engine_impl.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,14 @@ bool StorageEngineImpl::_handleInternalIdents(
331331
const std::string& ident,
332332
InternalIdentReconcilePolicy internalIdentReconcilePolicy,
333333
ReconcileResult* reconcileResult,
334-
std::set<std::string>* internalIdentsToDrop) {
334+
std::set<std::string>* internalIdentsToDrop,
335+
std::set<std::string>* allInternalIdents) {
335336
if (!_catalog->isInternalIdent(ident)) {
336337
return false;
337338
}
338339

340+
allInternalIdents->insert(ident);
341+
339342
if (InternalIdentReconcilePolicy::kDrop == internalIdentReconcilePolicy ||
340343
!supportsResumableIndexBuilds()) {
341344
internalIdentsToDrop->insert(ident);
@@ -378,8 +381,6 @@ bool StorageEngineImpl::_handleInternalIdents(
378381
reconcileResult->indexBuildsToResume.push_back(resumeInfo);
379382

380383
// Once we have parsed the resume info, we can safely drop the internal ident.
381-
// TODO SERVER-49846: revisit this logic since this could cause the side tables
382-
// associated with the index build to be orphaned if resuming fails.
383384
internalIdentsToDrop->insert(ident);
384385

385386
LOGV2(4916301,
@@ -435,6 +436,7 @@ StatusWith<StorageEngine::ReconcileResult> StorageEngineImpl::reconcileCatalogAn
435436
catalogIdents.insert(vec.begin(), vec.end());
436437
}
437438
std::set<std::string> internalIdentsToDrop;
439+
std::set<std::string> allInternalIdents;
438440

439441
auto dropPendingIdents = _dropPendingIdentReaper.getAllIdentNames();
440442

@@ -446,8 +448,12 @@ StatusWith<StorageEngine::ReconcileResult> StorageEngineImpl::reconcileCatalogAn
446448
continue;
447449
}
448450

449-
if (_handleInternalIdents(
450-
opCtx, it, internalIdentReconcilePolicy, &reconcileResult, &internalIdentsToDrop)) {
451+
if (_handleInternalIdents(opCtx,
452+
it,
453+
internalIdentReconcilePolicy,
454+
&reconcileResult,
455+
&internalIdentsToDrop,
456+
&allInternalIdents)) {
451457
continue;
452458
}
453459

@@ -623,6 +629,11 @@ StatusWith<StorageEngine::ReconcileResult> StorageEngineImpl::reconcileCatalogAn
623629
}
624630
}
625631

632+
// If there are no index builds to resume, we should drop all internal idents.
633+
if (reconcileResult.indexBuildsToResume.empty()) {
634+
internalIdentsToDrop.swap(allInternalIdents);
635+
}
636+
626637
for (auto&& temp : internalIdentsToDrop) {
627638
LOGV2(22257, "Dropping internal ident", "ident"_attr = temp);
628639
WriteUnitOfWork wuow(opCtx);

src/mongo/db/storage/storage_engine_impl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,8 @@ class StorageEngineImpl final : public StorageEngineInterface, public StorageEng
388388
const std::string& ident,
389389
InternalIdentReconcilePolicy internalIdentReconcilePolicy,
390390
ReconcileResult* reconcileResult,
391-
std::set<std::string>* internalIdentsToDrop);
391+
std::set<std::string>* internalIdentsToDrop,
392+
std::set<std::string>* allInternalIdents);
392393

393394
class RemoveDBChange;
394395

0 commit comments

Comments
 (0)