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
main
Cheng Chang 5 years ago committed by Facebook Github Bot
parent 6e97d4de00
commit a676001f95
  1. 25
      db/version_set.cc

@ -50,7 +50,6 @@
#include "table/two_level_iterator.h" #include "table/two_level_iterator.h"
#include "test_util/sync_point.h" #include "test_util/sync_point.h"
#include "util/coding.h" #include "util/coding.h"
#include "util/defer.h"
#include "util/stop_watch.h" #include "util/stop_watch.h"
#include "util/string_util.h" #include "util/string_util.h"
#include "util/user_comparator_wrapper.h" #include "util/user_comparator_wrapper.h"
@ -3614,14 +3613,6 @@ Status VersionSet::ProcessManifestWrites(
autovector<Version*> versions; autovector<Version*> versions;
autovector<const MutableCFOptions*> mutable_cf_options_ptrs; autovector<const MutableCFOptions*> mutable_cf_options_ptrs;
std::vector<std::unique_ptr<BaseReferencedVersionBuilder>> builder_guards; std::vector<std::unique_ptr<BaseReferencedVersionBuilder>> builder_guards;
Status s;
Defer defer([&s, &versions]() {
if (!s.ok()) {
for (auto v : versions) {
delete v;
}
}
});
if (first_writer.edit_list.front()->IsColumnFamilyManipulation()) { if (first_writer.edit_list.front()->IsColumnFamilyManipulation()) {
// No group commits for column family add or drop // No group commits for column family add or drop
@ -3707,8 +3698,12 @@ Status VersionSet::ProcessManifestWrites(
} else if (group_start != std::numeric_limits<size_t>::max()) { } else if (group_start != std::numeric_limits<size_t>::max()) {
group_start = std::numeric_limits<size_t>::max(); group_start = std::numeric_limits<size_t>::max();
} }
s = LogAndApplyHelper(last_writer->cfd, builder, e, mu); Status s = LogAndApplyHelper(last_writer->cfd, builder, e, mu);
if (!s.ok()) { if (!s.ok()) {
// free up the allocated memory
for (auto v : versions) {
delete v;
}
return s; return s;
} }
batch_edits.push_back(e); batch_edits.push_back(e);
@ -3718,8 +3713,12 @@ Status VersionSet::ProcessManifestWrites(
assert(!builder_guards.empty() && assert(!builder_guards.empty() &&
builder_guards.size() == versions.size()); builder_guards.size() == versions.size());
auto* builder = builder_guards[i]->version_builder(); 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()) { if (!s.ok()) {
// free up the allocated memory
for (auto v : versions) {
delete v;
}
return s; return s;
} }
} }
@ -3762,6 +3761,7 @@ Status VersionSet::ProcessManifestWrites(
#endif // NDEBUG #endif // NDEBUG
uint64_t new_manifest_file_size = 0; uint64_t new_manifest_file_size = 0;
Status s;
assert(pending_manifest_file_number_ == 0); assert(pending_manifest_file_number_ == 0);
if (!descriptor_log_ || if (!descriptor_log_ ||
@ -3968,6 +3968,9 @@ Status VersionSet::ProcessManifestWrites(
ROCKS_LOG_ERROR(db_options_->info_log, ROCKS_LOG_ERROR(db_options_->info_log,
"Error in committing version edit to MANIFEST: %s", "Error in committing version edit to MANIFEST: %s",
version_edits.c_str()); version_edits.c_str());
for (auto v : versions) {
delete v;
}
// If manifest append failed for whatever reason, the file could be // If manifest append failed for whatever reason, the file could be
// corrupted. So we need to force the next version update to start a // corrupted. So we need to force the next version update to start a
// new manifest file. // new manifest file.

Loading…
Cancel
Save