leveldb: No longer create/restore from backup tables.
ChromiumEnv made a copy of every leveldb table for all db's created for
Indexed DB. This was originally added for issue 239999 in r228952.
Today the corruption rate is low, and very few tables are ever restored
successfully, and may not improve the IDB corruption rate. Disabling
this feature which comes at the cost of doubling both disk usage, and
database compaction time (at least bytes written).
BUG=382272
Review-Url: https://codereview.chromium.org/2233973002
Cr-Commit-Position: refs/heads/master@{#411665}
diff --git a/third_party/leveldatabase/env_chromium_unittest.cc b/third_party/leveldatabase/env_chromium_unittest.cc
index 070e893..d4819e89 100644
--- a/third_party/leveldatabase/env_chromium_unittest.cc
+++ b/third_party/leveldatabase/env_chromium_unittest.cc
@@ -92,17 +92,10 @@
return false;
}
-class BackupEnv : public ChromiumEnv {
- public:
- BackupEnv() : ChromiumEnv("BackupEnv", true /* backup tables */) {}
-};
-
-base::LazyInstance<BackupEnv>::Leaky backup_env = LAZY_INSTANCE_INITIALIZER;
-
-TEST(ChromiumEnv, BackupTables) {
+TEST(ChromiumEnv, DeleteBackupTables) {
Options options;
options.create_if_missing = true;
- options.env = backup_env.Pointer();
+ options.env = Env::Default();
base::ScopedTempDir scoped_temp_dir;
ASSERT_TRUE(scoped_temp_dir.CreateUniqueTempDir());
@@ -115,36 +108,29 @@
EXPECT_TRUE(status.ok()) << status.ToString();
Slice a = "a";
Slice z = "z";
- db->CompactRange(&a, &z);
- int ldb_files = CountFilesWithExtension(dir, FPL(".ldb"));
- int bak_files = CountFilesWithExtension(dir, FPL(".bak"));
- EXPECT_GT(ldb_files, 0);
- EXPECT_EQ(ldb_files, bak_files);
- base::FilePath ldb_file;
- EXPECT_TRUE(GetFirstLDBFile(dir, &ldb_file));
+ db->CompactRange(&a, &z); // Ensure manifest written out to table.
delete db;
- EXPECT_TRUE(base::DeleteFile(ldb_file, false));
- EXPECT_EQ(ldb_files - 1, CountFilesWithExtension(dir, FPL(".ldb")));
+ db = nullptr;
- // The ldb file deleted above should be restored in Open.
- status = leveldb::DB::Open(options, dir.AsUTF8Unsafe(), &db);
+ // Current ChromiumEnv no longer makes backup tables - verify for sanity.
+ EXPECT_EQ(1, CountFilesWithExtension(dir, FPL(".ldb")));
+ EXPECT_EQ(0, CountFilesWithExtension(dir, FPL(".bak")));
+
+ // Manually create our own backup table to simulate opening db created by
+ // prior release.
+ base::FilePath ldb_path;
+ ASSERT_TRUE(GetFirstLDBFile(dir, &ldb_path));
+ base::FilePath bak_path = ldb_path.ReplaceExtension(FPL(".bak"));
+ ASSERT_TRUE(base::CopyFile(ldb_path, bak_path));
+ EXPECT_EQ(1, CountFilesWithExtension(dir, FPL(".bak")));
+
+ // Now reopen and close then verify the backup file was deleted.
+ status = DB::Open(options, dir.AsUTF8Unsafe(), &db);
EXPECT_TRUE(status.ok()) << status.ToString();
- std::string value;
- status = db->Get(ReadOptions(), "key", &value);
- EXPECT_TRUE(status.ok()) << status.ToString();
- EXPECT_EQ("value", value);
+ EXPECT_EQ(0, CountFilesWithExtension(dir, FPL(".bak")));
delete db;
-
- // Ensure that deleting an ldb file also deletes its backup.
- int orig_ldb_files = CountFilesWithExtension(dir, FPL(".ldb"));
- EXPECT_GT(ldb_files, 0);
- EXPECT_EQ(ldb_files, bak_files);
- EXPECT_TRUE(GetFirstLDBFile(dir, &ldb_file));
- options.env->DeleteFile(ldb_file.AsUTF8Unsafe());
- ldb_files = CountFilesWithExtension(dir, FPL(".ldb"));
- bak_files = CountFilesWithExtension(dir, FPL(".bak"));
- EXPECT_EQ(orig_ldb_files - 1, ldb_files);
- EXPECT_EQ(bak_files, ldb_files);
+ EXPECT_EQ(1, CountFilesWithExtension(dir, FPL(".ldb")));
+ EXPECT_EQ(0, CountFilesWithExtension(dir, FPL(".bak")));
}
TEST(ChromiumEnv, GetChildrenEmptyDir) {