SERVER-50289 Remove tempDir from persisted resumable index build state

8 files changed, 79 insertions, 67 deletions

diff --git a/jstests/noPassthrough/libs/index_build.js b/jstests/noPassthrough/libs/index_build.js
index ab82fbfe941..28f0b91fba0 100644
--- a/jstests/noPassthrough/libs/index_build.js
+++ b/jstests/noPassthrough/libs/index_build.js
@@ -620,7 +620,8 @@ const ResumableIndexBuildTest = class {
         if (!failWhileParsing) {
             // Ensure that the persisted Sorter data was cleaned up after failing to resume. This
             // cleanup does not occur if parsing failed.
-            assert.eq(listFiles(primary.dbpath + "/_tmp").length, 0);
+            const files = listFiles(primary.dbpath + "/_tmp");
+            assert.eq(files.length, 0, files);
 
             // If we fail after parsing, any remaining internal idents will only be cleaned up
             // after another restart.
diff --git a/src/mongo/db/catalog/multi_index_block.cpp b/src/mongo/db/catalog/multi_index_block.cpp
index 09a728ebced..617145e3abc 100644
--- a/src/mongo/db/catalog/multi_index_block.cpp
+++ b/src/mongo/db/catalog/multi_index_block.cpp
@@ -922,9 +922,6 @@ BSONObj MultiIndexBlock::_constructStateObject() const {
         if (_phase != IndexBuildPhaseEnum::kDrainWrites) {
             auto state = index.bulk->getPersistedSorterState();
 
-            // TODO (SERVER-50289): Consider not including tempDir in the persisted resumable index
-            // build state.
-            indexInfo.append("tempDir", state.tempDir);
             indexInfo.append("fileName", state.fileName);
             indexInfo.append("numKeys", index.bulk->getKeysInserted());
 
diff --git a/src/mongo/db/index/index_access_method.cpp b/src/mongo/db/index/index_access_method.cpp
index 4047fb9f705..cdc18cd1431 100644
--- a/src/mongo/db/index/index_access_method.cpp
+++ b/src/mongo/db/index/index_access_method.cpp
@@ -84,6 +84,13 @@ bool isMultikeyFromPaths(const MultikeyPaths& multikeyPaths) {
                        [](const MultikeyComponents& components) { return !components.empty(); });
 }
 
+SortOptions makeSortOptions(size_t maxMemoryUsageBytes) {
+    return SortOptions()
+        .TempDir(storageGlobalParams.dbpath + "/_tmp")
+        .ExtSortAllowed()
+        .MaxMemoryUsageBytes(maxMemoryUsageBytes);
+}
+
 }  // namespace
 
 struct BtreeExternalSortComparison {
@@ -495,10 +502,7 @@ std::unique_ptr<IndexAccessMethod::BulkBuilder> AbstractIndexAccessMethod::initi
 AbstractIndexAccessMethod::BulkBuilderImpl::BulkBuilderImpl(IndexCatalogEntry* index,
                                                             size_t maxMemoryUsageBytes)
     : _indexCatalogEntry(index),
-      _sorter(Sorter::make(SortOptions()
-                               .TempDir(storageGlobalParams.dbpath + "/_tmp")
-                               .ExtSortAllowed()
-                               .MaxMemoryUsageBytes(maxMemoryUsageBytes),
+      _sorter(Sorter::make(makeSortOptions(maxMemoryUsageBytes),
                            BtreeExternalSortComparison(),
                            _makeSorterSettings())) {}
 
@@ -508,10 +512,7 @@ AbstractIndexAccessMethod::BulkBuilderImpl::BulkBuilderImpl(IndexCatalogEntry* i
     : _indexCatalogEntry(index),
       _sorter(Sorter::makeFromExistingRanges(sorterInfo.getFileName()->toString(),
                                              *sorterInfo.getRanges(),
-                                             SortOptions()
-                                                 .TempDir(sorterInfo.getTempDir()->toString())
-                                                 .ExtSortAllowed()
-                                                 .MaxMemoryUsageBytes(maxMemoryUsageBytes),
+                                             makeSortOptions(maxMemoryUsageBytes),
                                              BtreeExternalSortComparison(),
                                              _makeSorterSettings())),
       _keysInserted(*sorterInfo.getNumKeys()) {}
diff --git a/src/mongo/db/index_builds_coordinator.cpp b/src/mongo/db/index_builds_coordinator.cpp
index 46ab4fc2191..d62ddd23692 100644
--- a/src/mongo/db/index_builds_coordinator.cpp
+++ b/src/mongo/db/index_builds_coordinator.cpp
@@ -1579,12 +1579,14 @@ void IndexBuildsCoordinator::restartIndexBuildsForRecovery(
                       "buildUUID"_attr = buildUUID);
 
                 boost::system::error_code ec;
-                boost::filesystem::remove(index.getFileName()->toString(), ec);
+                boost::filesystem::remove(
+                    storageGlobalParams.dbpath + "/_tmp/" + index.getFileName()->toString(), ec);
 
                 if (ec) {
                     LOGV2(5043101,
                           "Failed to remove resumable index build temp file",
                           "file"_attr = index.getFileName(),
+                          "buildUUID"_attr = buildUUID,
                           "error"_attr = ec.message());
                 }
             }
diff --git a/src/mongo/db/resumable_index_builds.idl b/src/mongo/db/resumable_index_builds.idl
index 74a53553100..bbe2f80dcd4 100644
--- a/src/mongo/db/resumable_index_builds.idl
+++ b/src/mongo/db/resumable_index_builds.idl
@@ -68,10 +68,6 @@ structs:
                               for this index build"
                 type: string
                 optional: true
-            tempDir:
-                description: "The directory into which we place a file when spilling data to disk"
-                type: string
-                optional: true
             fileName:
                 description: "The name of the file that sorted data is written to"
                 type: string
diff --git a/src/mongo/db/sorter/sorter.cpp b/src/mongo/db/sorter/sorter.cpp
index 514578b974d..c26d7aef88c 100644
--- a/src/mongo/db/sorter/sorter.cpp
+++ b/src/mongo/db/sorter/sorter.cpp
@@ -179,39 +179,40 @@ public:
         Settings;
     typedef std::pair<Key, Value> Data;
 
-    FileIterator(const std::string& fileName,
+    FileIterator(const std::string& fileFullPath,
                  std::streampos fileStartOffset,
                  std::streampos fileEndOffset,
                  const Settings& settings,
                  const uint32_t checksum)
         : _settings(settings),
           _done(false),
-          _fileName(fileName),
+          _fileFullPath(fileFullPath),
           _fileStartOffset(fileStartOffset),
           _fileEndOffset(fileEndOffset),
           _originalChecksum(checksum) {
         uassert(16815,
-                str::stream() << "unexpected empty file: " << _fileName,
-                boost::filesystem::file_size(_fileName) != 0);
+                str::stream() << "unexpected empty file: " << _fileFullPath,
+                boost::filesystem::file_size(_fileFullPath) != 0);
     }
 
     void openSource() {
-        _file.open(_fileName.c_str(), std::ios::in | std::ios::binary);
+        _file.open(_fileFullPath.c_str(), std::ios::in | std::ios::binary);
         uassert(16814,
-                str::stream() << "error opening file \"" << _fileName
+                str::stream() << "error opening file \"" << _fileFullPath
                               << "\": " << myErrnoWithDescription(),
                 _file.good());
         _file.seekg(_fileStartOffset);
         uassert(50979,
                 str::stream() << "error seeking starting offset of '" << _fileStartOffset
-                              << "' in file \"" << _fileName << "\": " << myErrnoWithDescription(),
+                              << "' in file \"" << _fileFullPath
+                              << "\": " << myErrnoWithDescription(),
                 _file.good());
     }
 
     void closeSource() {
         _file.close();
         uassert(50969,
-                str::stream() << "error closing file \"" << _fileName
+                str::stream() << "error closing file \"" << _fileFullPath
                               << "\": " << myErrnoWithDescription(),
                 !_file.fail());
 
@@ -339,7 +340,7 @@ private:
 
         const std::streampos offset = _file.tellg();
         uassert(51049,
-                str::stream() << "error reading file \"" << _fileName
+                str::stream() << "error reading file \"" << _fileFullPath
                               << "\": " << myErrnoWithDescription(),
                 offset >= 0);
 
@@ -351,7 +352,7 @@ private:
 
         _file.read(reinterpret_cast<char*>(out), size);
         uassert(16817,
-                str::stream() << "error reading file \"" << _fileName
+                str::stream() << "error reading file \"" << _fileFullPath
                               << "\": " << myErrnoWithDescription(),
                 _file.good());
         verify(_file.gcount() == static_cast<std::streamsize>(size));
@@ -362,7 +363,7 @@ private:
 
     std::unique_ptr<char[]> _buffer;
     std::unique_ptr<BufReader> _bufferReader;
-    std::string _fileName;            // File containing the sorted data range.
+    std::string _fileFullPath;        // File containing the sorted data range.
     std::streampos _fileStartOffset;  // File offset at which the sorted data range starts.
     std::streampos _fileEndOffset;    // File offset at which the sorted data range ends.
     std::ifstream _file;
@@ -390,14 +391,14 @@ public:
     typedef std::pair<Key, Value> Data;
 
     MergeIterator(const std::vector<std::shared_ptr<Input>>& iters,
-                  const std::string& itersSourceFileName,
+                  const std::string& itersSourceFileFullPath,
                   const SortOptions& opts,
                   const Comparator& comp)
         : _opts(opts),
           _remaining(opts.limit ? opts.limit : std::numeric_limits<unsigned long long>::max()),
           _first(true),
           _greater(comp),
-          _itersSourceFileName(itersSourceFileName) {
+          _itersSourceFileFullPath(itersSourceFileFullPath) {
         for (size_t i = 0; i < iters.size(); i++) {
             iters[i]->openSource();
             if (iters[i]->more()) {
@@ -423,7 +424,7 @@ public:
         // file. Some systems will error closing the file if any file handles are still open.
         _current.reset();
         _heap.clear();
-        DESTRUCTOR_GUARD(boost::filesystem::remove(_itersSourceFileName));
+        DESTRUCTOR_GUARD(boost::filesystem::remove(_itersSourceFileFullPath));
     }
 
     void openSource() {}
@@ -524,7 +525,7 @@ private:
     std::shared_ptr<Stream> _current;
     std::vector<std::shared_ptr<Stream>> _heap;  // MinHeap
     STLComparator _greater;                      // named so calls make sense
-    std::string _itersSourceFileName;
+    std::string _itersSourceFileFullPath;
 };
 
 template <typename Key, typename Value, typename Comparator>
@@ -561,7 +562,7 @@ public:
                        std::back_inserter(this->_iters),
                        [this](const SorterRange& range) {
                            return std::make_shared<sorter::FileIterator<Key, Value>>(
-                               this->_fileName,
+                               this->_fileFullPath,
                                range.getStartOffset(),
                                range.getEndOffset(),
                                this->_settings,
@@ -573,7 +574,7 @@ public:
         if (!_done && !this->_shouldKeepFilesOnDestruction) {
             // If done() was never called to return a MergeIterator, then this Sorter still owns
             // file deletion.
-            DESTRUCTOR_GUARD(boost::filesystem::remove(this->_fileName));
+            DESTRUCTOR_GUARD(boost::filesystem::remove(this->_fileFullPath));
         }
     }
 
@@ -610,7 +611,7 @@ public:
         }
 
         spill();
-        Iterator* mergeIt = Iterator::merge(this->_iters, this->_fileName, this->_opts, _comp);
+        Iterator* mergeIt = Iterator::merge(this->_iters, this->_fileFullPath, this->_opts, _comp);
         _done = true;
         return mergeIt;
     }
@@ -658,7 +659,7 @@ private:
         sort();
 
         SortedFileWriter<Key, Value> writer(
-            this->_opts, this->_fileName, _nextSortedFileWriterOffset, _settings);
+            this->_opts, this->_fileFullPath, _nextSortedFileWriterOffset, _settings);
         for (; !_data->empty(); _data->pop_front()) {
             writer.addAlreadySorted(_data->front().first, _data->front().second);
         }
@@ -760,7 +761,7 @@ public:
         if (!_done && !this->_shouldKeepFilesOnDestruction) {
             // If done() was never called to return a MergeIterator, then this Sorter still owns
             // file deletion.
-            DESTRUCTOR_GUARD(boost::filesystem::remove(this->_fileName));
+            DESTRUCTOR_GUARD(boost::filesystem::remove(this->_fileFullPath));
         }
     }
 
@@ -816,7 +817,7 @@ public:
         }
 
         spill();
-        Iterator* iterator = Iterator::merge(this->_iters, this->_fileName, this->_opts, _comp);
+        Iterator* iterator = Iterator::merge(this->_iters, this->_fileFullPath, this->_opts, _comp);
         _done = true;
         return iterator;
     }
@@ -947,7 +948,7 @@ private:
         updateCutoff();
 
         SortedFileWriter<Key, Value> writer(
-            this->_opts, this->_fileName, _nextSortedFileWriterOffset, _settings);
+            this->_opts, this->_fileFullPath, _nextSortedFileWriterOffset, _settings);
         for (size_t i = 0; i < _data.size(); i++) {
             writer.addAlreadySorted(_data[i].first, _data[i].second);
         }
@@ -984,7 +985,17 @@ private:
 
 template <typename Key, typename Value>
 Sorter<Key, Value>::Sorter(const SortOptions& opts)
-    : Sorter(opts, opts.extSortAllowed ? opts.tempDir + "/" + nextFileName() : "") {}
+    : _opts(opts),
+      _fileName(opts.extSortAllowed ? nextFileName() : ""),
+      _fileFullPath(opts.extSortAllowed ? opts.tempDir + "/" + _fileName : "") {}
+
+template <typename Key, typename Value>
+Sorter<Key, Value>::Sorter(const SortOptions& opts, const std::string& fileName)
+    : _opts(opts), _fileName(fileName), _fileFullPath(opts.tempDir + "/" + fileName) {
+    invariant(opts.extSortAllowed);
+    invariant(!opts.tempDir.empty());
+    invariant(!fileName.empty());
+}
 
 template <typename Key, typename Value>
 std::vector<SorterRange> Sorter<Key, Value>::_getRanges() const {
@@ -1010,10 +1021,15 @@ void Sorter<Key, Value>::persistDataForShutdown() {
 
 template <typename Key, typename Value>
 SortedFileWriter<Key, Value>::SortedFileWriter(const SortOptions& opts,
-                                               const std::string& fileName,
+                                               const std::string& fileFullPath,
                                                const std::streampos fileStartOffset,
                                                const Settings& settings)
-    : _settings(settings) {
+    : _settings(settings),
+      _fileFullPath(fileFullPath),
+      // The file descriptor is positioned at the end of a file when opened in append mode, but
+      // _file.tellp() is not initialized on all systems to reflect this. Therefore, we must also
+      // pass in the expected offset to this constructor.
+      _fileStartOffset(fileStartOffset) {
 
     // This should be checked by consumers, but if we get here don't allow writes.
     uassert(
@@ -1025,20 +1041,14 @@ SortedFileWriter<Key, Value>::SortedFileWriter(const SortOptions& opts,
 
     boost::filesystem::create_directories(opts.tempDir);
 
-    _fileName = fileName;
-
     // We open the provided file in append mode so that SortedFileWriter instances can share the
     // same file, used serially. We want to share files in order to stay below system open file
     // limits.
-    _file.open(_fileName.c_str(), std::ios::binary | std::ios::app | std::ios::out);
+    _file.open(_fileFullPath.c_str(), std::ios::binary | std::ios::app | std::ios::out);
     uassert(16818,
-            str::stream() << "error opening file \"" << _fileName
+            str::stream() << "error opening file \"" << _fileFullPath
                           << "\": " << sorter::myErrnoWithDescription(),
             _file.good());
-    // The file descriptor is positioned at the end of a file when opened in append mode, but
-    // _file.tellp() is not initialized on all systems to reflect this. Therefore, we must also pass
-    // in the expected offset to this constructor.
-    _fileStartOffset = fileStartOffset;
 
     // throw on failure
     _file.exceptions(std::ios::failbit | std::ios::badbit | std::ios::eofbit);
@@ -1106,7 +1116,7 @@ void SortedFileWriter<Key, Value>::spill() {
         _file.write(outBuffer, std::abs(size));
     } catch (const std::exception&) {
         msgasserted(16821,
-                    str::stream() << "error writing to file \"" << _fileName
+                    str::stream() << "error writing to file \"" << _fileFullPath
                                   << "\": " << sorter::myErrnoWithDescription());
     }
 
@@ -1118,8 +1128,8 @@ SortIteratorInterface<Key, Value>* SortedFileWriter<Key, Value>::done() {
     spill();
     std::streampos currentFileOffset = _file.tellp();
     uassert(50980,
-            str::stream() << "error fetching current file descriptor offset in file \"" << _fileName
-                          << "\": " << sorter::myErrnoWithDescription(),
+            str::stream() << "error fetching current file descriptor offset in file \""
+                          << _fileFullPath << "\": " << sorter::myErrnoWithDescription(),
             currentFileOffset >= 0);
 
     // In case nothing was written to disk, use _fileStartOffset because tellp() may not be
@@ -1128,7 +1138,7 @@ SortIteratorInterface<Key, Value>* SortedFileWriter<Key, Value>::done() {
     _file.close();
 
     return new sorter::FileIterator<Key, Value>(
-        _fileName, _fileStartOffset, _fileEndOffset, _settings, _checksum);
+        _fileFullPath, _fileStartOffset, _fileEndOffset, _settings, _checksum);
 }
 
 //
@@ -1139,10 +1149,10 @@ template <typename Key, typename Value>
 template <typename Comparator>
 SortIteratorInterface<Key, Value>* SortIteratorInterface<Key, Value>::merge(
     const std::vector<std::shared_ptr<SortIteratorInterface>>& iters,
-    const std::string& fileName,
+    const std::string& fileFullPath,
     const SortOptions& opts,
     const Comparator& comp) {
-    return new sorter::MergeIterator<Key, Value, Comparator>(iters, fileName, opts, comp);
+    return new sorter::MergeIterator<Key, Value, Comparator>(iters, fileFullPath, opts, comp);
 }
 
 template <typename Key, typename Value>
diff --git a/src/mongo/db/sorter/sorter.h b/src/mongo/db/sorter/sorter.h
index d301c4c6205..f4a9869bd54 100644
--- a/src/mongo/db/sorter/sorter.h
+++ b/src/mongo/db/sorter/sorter.h
@@ -178,7 +178,7 @@ public:
     template <typename Comparator>
     static SortIteratorInterface* merge(
         const std::vector<std::shared_ptr<SortIteratorInterface>>& iters,
-        const std::string& fileName,
+        const std::string& fileFullPath,
         const SortOptions& opts,
         const Comparator& comp);
 
@@ -223,15 +223,16 @@ public:
         Settings;
 
     struct PersistedState {
-        std::string tempDir;
         std::string fileName;
         std::vector<SorterRange> ranges;
     };
 
-    Sorter(const SortOptions& opts);
+    explicit Sorter(const SortOptions& opts);
 
-    Sorter(const SortOptions& opts, const std::string& fileName)
-        : _opts(opts), _fileName(fileName) {}
+    /**
+     * ExtSort-only constructor. fileName is the base name of a file in the temp directory.
+     */
+    Sorter(const SortOptions& opts, const std::string& fileName);
 
     template <typename Comparator>
     static Sorter* make(const SortOptions& opts,
@@ -263,7 +264,7 @@ public:
     }
 
     PersistedState getPersistedState() const {
-        return {_opts.tempDir, _fileName, _getRanges()};
+        return {_fileName, _getRanges()};
     }
 
     void persistDataForShutdown();
@@ -281,8 +282,13 @@ protected:
     bool _shouldKeepFilesOnDestruction = false;
 
     SortOptions _opts;
+
+    // Used by Sorter::getPersistedState() to return the base file name of the data persisted by
+    // this Sorter.
     std::string _fileName;
 
+    std::string _fileFullPath;
+
     std::vector<std::shared_ptr<Iterator>> _iters;  // Data that has already been spilled.
 };
 
@@ -302,7 +308,7 @@ public:
         Settings;
 
     explicit SortedFileWriter(const SortOptions& opts,
-                              const std::string& fileName,
+                              const std::string& fileFullPath,
                               const std::streampos fileStartOffset,
                               const Settings& settings = Settings());
 
@@ -328,7 +334,7 @@ private:
     void spill();
 
     const Settings _settings;
-    std::string _fileName;
+    std::string _fileFullPath;
     std::ofstream _file;
     BufBuilder _buffer;
 
@@ -364,7 +370,7 @@ private:
     template ::mongo::SortIteratorInterface<Key, Value>* ::mongo::                             \
         SortIteratorInterface<Key, Value>::merge<Comparator>(                                  \
             const std::vector<std::shared_ptr<SortIteratorInterface>>& iters,                  \
-            const std::string& fileName,                                                       \
+            const std::string& fileFullPath,                                                   \
             const SortOptions& opts,                                                           \
             const Comparator& comp);                                                           \
     template ::mongo::Sorter<Key, Value>* ::mongo::Sorter<Key, Value>::make<Comparator>(       \
diff --git a/src/mongo/db/sorter/sorter_test.cpp b/src/mongo/db/sorter/sorter_test.cpp
index 4de5eb031f8..4000ceab68c 100644
--- a/src/mongo/db/sorter/sorter_test.cpp
+++ b/src/mongo/db/sorter/sorter_test.cpp
@@ -482,8 +482,7 @@ private:
     void assertRangeInfo(unowned_ptr<IWSorter> sorter, const SortOptions& opts) {
         auto state = sorter->getPersistedState();
         if (opts.extSortAllowed) {
-            ASSERT_EQ(state.tempDir, opts.tempDir);
-            ASSERT_STRING_CONTAINS(state.fileName, opts.tempDir);
+            ASSERT_NE(state.fileName, "");
         }
         if (auto numRanges = correctNumRanges()) {
             ASSERT_EQ(state.ranges.size(), *numRanges);