fix(compaction): reject a non-empty or missing --working-dir before destructive steps

- the working directory is consumed by the command (renamed onto dblogdir on a
  real run, removed on a dry run), so a mistaken --working-dir pointing at a
  directory that holds data would destroy the user's files
- require an existing empty directory and exit with status 64 otherwise, before
  any destructive step, consistent with the cross-filesystem check
- add regression tests for the non-empty and non-existent cases, verifying the
  log directory (and the working directory) are left intact
- document the requirement in the --working-dir description and the exit-status
  64 list

Author: umegane

docs/tglogutil-compaction-man.md

@@ -25,7 +25,8 @@ Options:
 * `--thread-num=<number>`
     * Number (default `1`) of concurrent processing thread of reading log files
 * `--working-dir=</path/to/working-dir>`
-    * Directory for storing temporary files (default is a uniquely named directory next to `dblogdir`)
+    * Directory used as the working directory itself (not a parent under which one is created). By default a uniquely named directory is created next to `dblogdir`.
+    * Must be an existing empty directory dedicated to this command. Its contents are consumed by the command: on a real run the directory is renamed onto `dblogdir` at the end, and on a dry run it is removed on completion. If the path does not exist or is not empty, the command stops before any destructive operation and exits with status 64. Do not point it at a directory whose contents you want to keep.
     * Must be on the same filesystem as `dblogdir`. If it is on a different filesystem, the command stops before any destructive operation and exits with status 64.
 * `--verbose=<bool>`
     * Verbose mode (default `false`)
@@ -48,6 +49,7 @@ Options:
         * Specified a directory that is not the transaction log directory
         * Specified a transaction log directory of unsupported format version
         * `epoch` file does not exist
+    * `working-dir` does not exist or is not an empty directory
     * `working-dir` is on a different filesystem than `dblogdir`
     * files in `dblogdir` are damaged
 

src/limestone/dblogutil/dblogutil.cpp

@@ -369,7 +369,21 @@ void compaction(dblog_scan &ds) {
     boost::filesystem::path tmp;
     if (!FLAGS_working_dir.empty()) {
         tmp = FLAGS_working_dir;
-        // TODO: check, error if exist and non-empty
+        // The working directory is consumed by the command: on a real run it is
+        // renamed onto from_dir, and on a dry run it is removed. Require an existing
+        // empty directory so that a mistaken --working-dir never destroys the user's
+        // files. A non-existent path or a non-empty directory is rejected before any
+        // destructive step.
+        boost::system::error_code ec;
+        if (!boost::filesystem::is_directory(tmp, ec)) {
+            LOG(ERROR) << "working directory must be an existing directory: "
+                       << "working-directory=" << tmp;
+            log_and_exit(64);
+        }
+        if (!boost::filesystem::is_empty(tmp, ec)) {
+            LOG(ERROR) << "working directory must be empty: working-directory=" << tmp;
+            log_and_exit(64);
+        }
     } else {
         tmp = make_work_dir_next_to(from_dir);
     }

test/limestone/compaction/offline_compaction_test.cpp

@@ -600,6 +600,75 @@ TEST_F(offline_compaction_test, offline_compaction_ignores_malformed_epoch_optio
     EXPECT_EQ(kv_list[0].second, "va");
 }
 
+// --working-dir is consumed by the command (renamed onto dblogdir on a real run,
+// removed on a dry run), so pointing it at a non-empty directory would destroy the
+// user's data. Such a directory must be rejected up front, before any destructive
+// step, leaving the original log directory intact.
+TEST_F(offline_compaction_test, offline_compaction_rejects_non_empty_working_dir) {
+    gen_datastore();
+    datastore_->switch_epoch(1);
+    lc0_->begin_session();
+    lc0_->add_entry(1, "A", "va", {1, 0});
+    lc0_->end_session();
+    datastore_->switch_epoch(2);
+    datastore_->shutdown();
+    datastore_ = nullptr;
+
+    std::map<std::string, std::string> before = snapshot_tree(location);
+
+    // A non-empty working directory on the same filesystem as the log directory.
+    boost::filesystem::path working_dir =
+        boost::filesystem::path(test_root_) / "non_empty_wd";
+    boost::filesystem::create_directories(working_dir);
+    {
+        std::ofstream ofs((working_dir / "precious.txt").string());
+        ofs << "do not delete me";
+    }
+
+    std::string out;
+    std::string command = std::string(util_command) + " compaction --force --working_dir=" +
+        working_dir.string() + " " + std::string(location) + " 2>&1";
+    int rc = invoke(command, out);
+    EXPECT_NE(rc, 0) << "compaction should fail on a non-empty working directory";
+    EXPECT_NE(out.find("working directory must be empty"), std::string::npos)
+        << "tglogutil output:\n" << out;
+
+    // The rejection must leave both the log directory and the working directory intact.
+    EXPECT_EQ(snapshot_tree(location), before);
+    EXPECT_TRUE(boost::filesystem::exists(working_dir / "precious.txt"));
+
+    boost::filesystem::remove_all(working_dir);
+}
+
+// --working-dir must be an existing empty directory. A non-existent path is rejected
+// up front, leaving the original log directory intact.
+TEST_F(offline_compaction_test, offline_compaction_rejects_non_existent_working_dir) {
+    gen_datastore();
+    datastore_->switch_epoch(1);
+    lc0_->begin_session();
+    lc0_->add_entry(1, "A", "va", {1, 0});
+    lc0_->end_session();
+    datastore_->switch_epoch(2);
+    datastore_->shutdown();
+    datastore_ = nullptr;
+
+    std::map<std::string, std::string> before = snapshot_tree(location);
+
+    boost::filesystem::path working_dir =
+        boost::filesystem::path(test_root_) / "missing_wd";
+    ASSERT_FALSE(boost::filesystem::exists(working_dir));
+
+    std::string out;
+    std::string command = std::string(util_command) + " compaction --force --working_dir=" +
+        working_dir.string() + " " + std::string(location) + " 2>&1";
+    int rc = invoke(command, out);
+    EXPECT_NE(rc, 0) << "compaction should fail on a non-existent working directory";
+    EXPECT_NE(out.find("working directory must be an existing directory"), std::string::npos)
+        << "tglogutil output:\n" << out;
+
+    EXPECT_EQ(snapshot_tree(location), before);
+}
+
 // With --make_backup, the blob directory must be copied (not moved) so that both the
 // compacted log directory and the backup directory keep the blob data.
 TEST_F(offline_compaction_test, offline_compaction_with_backup_copies_blob_directory) {