From a676001f95f9b61f96fb5fc68f2c7efb1758bffd Mon Sep 17 00:00:00 2001 From: Cheng Chang Date: Thu, 13 Feb 2020 10:18:23 -0800 Subject: [PATCH] Revert usage of Defer. (#6410) Summary: Seems like this caused the following test failure on AppVeyor: DBTest2.CrashInRecoveryMultipleCF c:\projects\rocksdb\db\db_test_util.cc(107): error: DestroyDB(dbname_, options) IO error: Failed to delete: C:\projects\rocksdb\db_tests\\testrocksdb-3112//db_test2_10791409581227174103/000013.sst: Access is denied. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6410 Test Plan: Wait to see whether the AppVeyor test passes. Differential Revision: D19879872 Pulled By: cheng-chang fbshipit-source-id: 59a9c55ca88566e9210c0b715ecc45a4fd9afe26 --- db/version_set.cc | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/db/version_set.cc b/db/version_set.cc index 794060d8a3..051fe2ef7f 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -50,7 +50,6 @@ #include "table/two_level_iterator.h" #include "test_util/sync_point.h" #include "util/coding.h" -#include "util/defer.h" #include "util/stop_watch.h" #include "util/string_util.h" #include "util/user_comparator_wrapper.h" @@ -3614,14 +3613,6 @@ Status VersionSet::ProcessManifestWrites( autovector versions; autovector mutable_cf_options_ptrs; std::vector> builder_guards; - Status s; - Defer defer([&s, &versions]() { - if (!s.ok()) { - for (auto v : versions) { - delete v; - } - } - }); if (first_writer.edit_list.front()->IsColumnFamilyManipulation()) { // No group commits for column family add or drop @@ -3707,8 +3698,12 @@ Status VersionSet::ProcessManifestWrites( } else if (group_start != std::numeric_limits::max()) { group_start = std::numeric_limits::max(); } - s = LogAndApplyHelper(last_writer->cfd, builder, e, mu); + Status s = LogAndApplyHelper(last_writer->cfd, builder, e, mu); if (!s.ok()) { + // free up the allocated memory + for (auto v : versions) { + delete v; + } return s; } batch_edits.push_back(e); @@ -3718,8 +3713,12 @@ Status VersionSet::ProcessManifestWrites( assert(!builder_guards.empty() && builder_guards.size() == versions.size()); auto* builder = builder_guards[i]->version_builder(); - s = builder->SaveTo(versions[i]->storage_info()); + Status s = builder->SaveTo(versions[i]->storage_info()); if (!s.ok()) { + // free up the allocated memory + for (auto v : versions) { + delete v; + } return s; } } @@ -3762,6 +3761,7 @@ Status VersionSet::ProcessManifestWrites( #endif // NDEBUG uint64_t new_manifest_file_size = 0; + Status s; assert(pending_manifest_file_number_ == 0); if (!descriptor_log_ || @@ -3968,6 +3968,9 @@ Status VersionSet::ProcessManifestWrites( ROCKS_LOG_ERROR(db_options_->info_log, "Error in committing version edit to MANIFEST: %s", version_edits.c_str()); + for (auto v : versions) { + delete v; + } // If manifest append failed for whatever reason, the file could be // corrupted. So we need to force the next version update to start a // new manifest file.