SERVER-20958: Ensure file names are unique when quickly written and rotated.

3 files changed, 39 insertions, 16 deletions

diff --git a/src/mongo/db/ftdc/file_manager.cpp b/src/mongo/db/ftdc/file_manager.cpp
index 7acfc03692a..aed804df97c 100644
--- a/src/mongo/db/ftdc/file_manager.cpp
+++ b/src/mongo/db/ftdc/file_manager.cpp
@@ -88,7 +88,7 @@ StatusWith<std::unique_ptr<FTDCFileManager>> FTDCFileManager::create(
     auto interimDocs = mgr->recoverInterimFile();
 
     // Open the archive file for writing
-    auto swFile = generateArchiveFileName(path, terseUTCCurrentTime());
+    auto swFile = mgr->generateArchiveFileName(path, terseUTCCurrentTime());
     if (!swFile.isOK()) {
         return swFile.getStatus();
     }
@@ -131,14 +131,18 @@ StatusWith<boost::filesystem::path> FTDCFileManager::generateArchiveFileName(
     fileName += std::string(".");
     fileName += suffix.toString();
 
+    if (_previousArchiveFileSuffix != suffix) {
+        // If the suffix has changed, reset the uniquifier counter to zero
+        _previousArchiveFileSuffix = suffix.toString();
+        _fileNameUniquifier = 0;
+    }
+
     if (boost::filesystem::exists(path)) {
-        // TODO: keep track of a high watermark for the current suffix so that after rotate the
-        // counter does not reset.
-        for (std::uint32_t i = 0; i < FTDCConfig::kMaxFileUniqifier; ++i) {
+        for (; _fileNameUniquifier < FTDCConfig::kMaxFileUniqifier; ++_fileNameUniquifier) {
             char buf[20];
 
             // Use leading zeros so the numbers sort lexigraphically
-            int ret = snprintf(&buf[0], sizeof(buf), "%05u", i);
+            int ret = snprintf(&buf[0], sizeof(buf), "%05u", _fileNameUniquifier);
             invariant(ret > 0 && ret < static_cast<int>((sizeof(buf) - 1)));
 
             auto fileNameUnique = fileName;
@@ -206,11 +210,12 @@ void FTDCFileManager::trimDirectory(std::vector<boost::filesystem::path>& files)
     dassert(std::is_sorted(files.begin(), files.end()));
 
     for (auto it = files.rbegin(); it != files.rend(); ++it) {
-        size += boost::filesystem::file_size(*it);
+        std::uint64_t fileSize = boost::filesystem::file_size(*it);
+        size += fileSize;
 
         if (size >= maxSize) {
             LOG(1) << "Cleaning file over full-time diagnostic data capture quota, file: "
-                   << (*it).generic_string();
+                   << (*it).generic_string() << " with size " << fileSize;
             boost::filesystem::remove(*it);
         }
     }
diff --git a/src/mongo/db/ftdc/file_manager.h b/src/mongo/db/ftdc/file_manager.h
index 3342a1877cf..acfd330d581 100644
--- a/src/mongo/db/ftdc/file_manager.h
+++ b/src/mongo/db/ftdc/file_manager.h
@@ -94,8 +94,8 @@ public:
      * Generate a new file name for the archive.
      * Public for use by unit tests only.
      */
-    static StatusWith<boost::filesystem::path> generateArchiveFileName(
-        const boost::filesystem::path& path, StringData suffix);
+    StatusWith<boost::filesystem::path> generateArchiveFileName(const boost::filesystem::path& path,
+                                                                StringData suffix);
 
 private:
     FTDCFileManager(const FTDCConfig* config,
@@ -137,6 +137,13 @@ private:
     // file to log samples to
     FTDCFileWriter _writer;
 
+    // last archive file name suffix used
+    std::string _previousArchiveFileSuffix;
+
+    // last file name id uniquifier used
+    // this starts from zero for each new file suffix
+    std::uint32_t _fileNameUniquifier = 0;
+
     // Path of metrics directory
     boost::filesystem::path _path;
 
diff --git a/src/mongo/db/ftdc/file_manager_test.cpp b/src/mongo/db/ftdc/file_manager_test.cpp
index 3c1088ed547..636fbb6b193 100644
--- a/src/mongo/db/ftdc/file_manager_test.cpp
+++ b/src/mongo/db/ftdc/file_manager_test.cpp
@@ -104,12 +104,14 @@ TEST(FTDCFileManagerTest, TestFull) {
     for (auto& file : files) {
         int fs = boost::filesystem::file_size(file);
         ASSERT_TRUE(fs < c.maxFileSizeBytes * 1.10);
+        unittest::log() << "File " << file.generic_string() << " has size " << fs;
         if (file.generic_string().find("interim") == std::string::npos) {
             sum += fs;
         }
     }
 
-    ASSERT_TRUE(sum < c.maxDirectorySizeBytes * 1.10 && sum > c.maxDirectorySizeBytes * 0.90);
+    ASSERT_LESS_THAN_OR_EQUALS(sum, c.maxDirectorySizeBytes * 1.10);
+    ASSERT_GREATER_THAN_OR_EQUALS(sum, c.maxDirectorySizeBytes * 0.90);
 }
 
 void ValidateInterimFileHasData(const boost::filesystem::path& dir, bool hasData) {
@@ -238,7 +240,7 @@ TEST(FTDCFileManagerTest, TestCorruptCrashRestart) {
 
         mgr->close();
 
-        auto swFile = FTDCFileManager::generateArchiveFileName(dir, "0test-crash");
+        auto swFile = mgr->generateArchiveFileName(dir, "0test-crash");
         ASSERT_OK(swFile);
 
         std::ofstream stream(swFile.getValue().c_str());
@@ -261,8 +263,6 @@ TEST(FTDCFileManagerTest, TestNormalCrashInterim) {
     unittest::TempDir tempdir("metrics_testpath");
     boost::filesystem::path dir(tempdir.path());
 
-    createDirectoryClean(dir);
-
     BSONObj mdoc1 = BSON("name"
                          << "some_metadata"
                          << "key1" << 34 << "something" << 98);
@@ -274,13 +274,24 @@ TEST(FTDCFileManagerTest, TestNormalCrashInterim) {
                          << "joe"
                          << "key3" << 34 << "key5" << 45);
 
-    auto swFile = FTDCFileManager::generateArchiveFileName(dir, "0test-crash");
-    ASSERT_OK(swFile);
+    boost::filesystem::path fileOut;
+
+    {
+        FTDCCollectorCollection rotate;
+        auto swMgr = FTDCFileManager::create(&c, dir, &rotate, client);
+        ASSERT_OK(swMgr.getStatus());
+        auto swFile = swMgr.getValue()->generateArchiveFileName(dir, "0test-crash");
+        ASSERT_OK(swFile);
+        fileOut = swFile.getValue();
+        ASSERT_OK(swMgr.getValue()->close());
+    }
+
+    createDirectoryClean(dir);
 
     {
         FTDCFileWriter writer(&c);
 
-        ASSERT_OK(writer.open(swFile.getValue()));
+        ASSERT_OK(writer.open(fileOut));
 
         ASSERT_OK(writer.writeMetadata(mdoc1));