summaryrefslogtreecommitdiffstats
path: root/sync/syncable
diff options
context:
space:
mode:
authorstanisc <stanisc@chromium.org>2015-03-16 21:40:19 -0700
committerCommit bot <commit-bot@chromium.org>2015-03-17 04:40:48 +0000
commita3ed68bbb72ec9fdf16d866f41cb0951d006088b (patch)
treef98fe2e5bf32c5652167782e64967a04a8fa0a96 /sync/syncable
parent69ca66410b63d9fd7ce1ef620541783233349362 (diff)
downloadchromium_src-a3ed68bbb72ec9fdf16d866f41cb0951d006088b.zip
chromium_src-a3ed68bbb72ec9fdf16d866f41cb0951d006088b.tar.gz
chromium_src-a3ed68bbb72ec9fdf16d866f41cb0951d006088b.tar.bz2
Sync: Avoid 3 passes over SyncDarta DB when loading the directory from the disk
According to profiling the first two read passes over SyncData DB are from DirectoryBackingStore::DropDeletedEntries. This function executes two SQL queries that drop entries pending deletion. Since it is uncommon to have entries pending deletion it is far more efficient to remove DirectoryBackingStore::DropDeletedEntries and instead check for the entries matching this criteria inside DirectoryBackingStore::LoadEntries, skip those entries from being loaded and put them straight into Directory's metahandles_to_purge collection which sets them up for deletion during a subsequent database save. See the bug for the preliminary performance results. BUG=464073 Review URL: https://codereview.chromium.org/1008103002 Cr-Commit-Position: refs/heads/master@{#320861}
Diffstat (limited to 'sync/syncable')
-rw-r--r--sync/syncable/deferred_on_disk_directory_backing_store.cc3
-rw-r--r--sync/syncable/deferred_on_disk_directory_backing_store.h1
-rw-r--r--sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc15
-rw-r--r--sync/syncable/directory.cc12
-rw-r--r--sync/syncable/directory.h5
-rw-r--r--sync/syncable/directory_backing_store.cc35
-rw-r--r--sync/syncable/directory_backing_store.h17
-rw-r--r--sync/syncable/directory_backing_store_unittest.cc47
-rw-r--r--sync/syncable/directory_unittest.cc81
-rw-r--r--sync/syncable/in_memory_directory_backing_store.cc5
-rw-r--r--sync/syncable/in_memory_directory_backing_store.h1
-rw-r--r--sync/syncable/invalid_directory_backing_store.cc1
-rw-r--r--sync/syncable/invalid_directory_backing_store.h1
-rw-r--r--sync/syncable/mutable_entry.cc6
-rw-r--r--sync/syncable/on_disk_directory_backing_store.cc11
-rw-r--r--sync/syncable/on_disk_directory_backing_store.h9
16 files changed, 179 insertions, 71 deletions
diff --git a/sync/syncable/deferred_on_disk_directory_backing_store.cc b/sync/syncable/deferred_on_disk_directory_backing_store.cc
index 0367e5b..fbb7845 100644
--- a/sync/syncable/deferred_on_disk_directory_backing_store.cc
+++ b/sync/syncable/deferred_on_disk_directory_backing_store.cc
@@ -52,6 +52,7 @@ bool DeferredOnDiskDirectoryBackingStore::SaveChanges(
DirOpenResult DeferredOnDiskDirectoryBackingStore::Load(
Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) {
// Open an in-memory database at first to create initial sync data needed by
// Directory.
@@ -61,7 +62,7 @@ DirOpenResult DeferredOnDiskDirectoryBackingStore::Load(
if (!InitializeTables())
return FAILED_OPEN_DATABASE;
- if (!LoadEntries(handles_map))
+ if (!LoadEntries(handles_map, metahandles_to_purge))
return FAILED_DATABASE_CORRUPT;
if (!LoadInfo(kernel_load_info))
return FAILED_DATABASE_CORRUPT;
diff --git a/sync/syncable/deferred_on_disk_directory_backing_store.h b/sync/syncable/deferred_on_disk_directory_backing_store.h
index c697120..b50da38 100644
--- a/sync/syncable/deferred_on_disk_directory_backing_store.h
+++ b/sync/syncable/deferred_on_disk_directory_backing_store.h
@@ -25,6 +25,7 @@ class SYNC_EXPORT_PRIVATE DeferredOnDiskDirectoryBackingStore
~DeferredOnDiskDirectoryBackingStore() override;
DirOpenResult Load(Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) override;
bool SaveChanges(const Directory::SaveChangesSnapshot& snapshot) override;
diff --git a/sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc b/sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc
index fbba75e..020eeb4 100644
--- a/sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc
+++ b/sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc
@@ -31,6 +31,7 @@ class DeferredOnDiskDirectoryBackingStoreTest : public testing::Test {
base::FilePath db_path_;
Directory::MetahandlesMap handles_map_;
JournalIndex delete_journals_;
+ MetahandleSet metahandles_to_purge_;
Directory::KernelLoadInfo kernel_load_info_;
};
@@ -38,8 +39,9 @@ class DeferredOnDiskDirectoryBackingStoreTest : public testing::Test {
TEST_F(DeferredOnDiskDirectoryBackingStoreTest, Load) {
DeferredOnDiskDirectoryBackingStore store("test", db_path_);
EXPECT_EQ(OPENED, store.Load(&handles_map_, &delete_journals_,
- &kernel_load_info_));
+ &metahandles_to_purge_, &kernel_load_info_));
EXPECT_TRUE(delete_journals_.empty());
+ EXPECT_TRUE(metahandles_to_purge_.empty());
ASSERT_EQ(1u, handles_map_.size()); // root node
ASSERT_TRUE(handles_map_.count(1));
EntryKernel* root = handles_map_[1];
@@ -55,7 +57,7 @@ TEST_F(DeferredOnDiskDirectoryBackingStoreTest,
// Open and close.
DeferredOnDiskDirectoryBackingStore store("test", db_path_);
EXPECT_EQ(OPENED, store.Load(&handles_map_, &delete_journals_,
- &kernel_load_info_));
+ &metahandles_to_purge_, &kernel_load_info_));
}
EXPECT_FALSE(base::PathExists(db_path_));
@@ -68,7 +70,7 @@ TEST_F(DeferredOnDiskDirectoryBackingStoreTest,
// Open and close.
DeferredOnDiskDirectoryBackingStore store("test", db_path_);
EXPECT_EQ(OPENED, store.Load(&handles_map_, &delete_journals_,
- &kernel_load_info_));
+ &metahandles_to_purge_, &kernel_load_info_));
Directory::SaveChangesSnapshot snapshot;
store.SaveChanges(snapshot);
@@ -83,7 +85,7 @@ TEST_F(DeferredOnDiskDirectoryBackingStoreTest, PersistWhenSavingValidChanges) {
// Open and close.
DeferredOnDiskDirectoryBackingStore store("test", db_path_);
EXPECT_EQ(OPENED, store.Load(&handles_map_, &delete_journals_,
- &kernel_load_info_));
+ &metahandles_to_purge_, &kernel_load_info_));
Directory::SaveChangesSnapshot snapshot;
EntryKernel* entry = new EntryKernel();
@@ -98,8 +100,9 @@ TEST_F(DeferredOnDiskDirectoryBackingStoreTest, PersistWhenSavingValidChanges) {
ASSERT_TRUE(base::PathExists(db_path_));
OnDiskDirectoryBackingStore read_store("test", db_path_);
- EXPECT_EQ(OPENED, read_store.Load(&handles_map_, &delete_journals_,
- &kernel_load_info_));
+ EXPECT_EQ(OPENED,
+ read_store.Load(&handles_map_, &delete_journals_,
+ &metahandles_to_purge_, &kernel_load_info_));
ASSERT_EQ(2u, handles_map_.size());
ASSERT_TRUE(handles_map_.count(1)); // root node
ASSERT_TRUE(handles_map_.count(2));
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 1261695..9a689ac 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -77,7 +77,8 @@ Directory::SaveChangesSnapshot::~SaveChangesSnapshot() {
Directory::Kernel::Kernel(
const std::string& name,
- const KernelLoadInfo& info, DirectoryChangeDelegate* delegate,
+ const KernelLoadInfo& info,
+ DirectoryChangeDelegate* delegate,
const WeakHandle<TransactionObserver>& transaction_observer)
: next_write_transaction_id(0),
name(name),
@@ -178,12 +179,13 @@ DirOpenResult Directory::OpenImpl(
// Avoids mem leaks on failure. Harmlessly deletes the empty hash map after
// the swap in the success case.
- STLValueDeleter<Directory::MetahandlesMap> deleter(&tmp_handles_map);
+ STLValueDeleter<MetahandlesMap> deleter(&tmp_handles_map);
JournalIndex delete_journals;
+ MetahandleSet metahandles_to_purge;
- DirOpenResult result =
- store_->Load(&tmp_handles_map, &delete_journals, &info);
+ DirOpenResult result = store_->Load(&tmp_handles_map, &delete_journals,
+ &metahandles_to_purge, &info);
if (OPENED != result)
return result;
@@ -195,6 +197,8 @@ DirOpenResult Directory::OpenImpl(
// prevent local ID reuse in the case of an early crash. See the comments in
// TakeSnapshotForSaveChanges() or crbug.com/142987 for more information.
kernel_->info_status = KERNEL_SHARE_INFO_DIRTY;
+
+ kernel_->metahandles_to_purge.swap(metahandles_to_purge);
if (!SaveChanges())
return FAILED_INITIAL_WRITE;
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index 98706b6..89eec17 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -599,11 +599,12 @@ class SYNC_EXPORT Directory {
// processed |snapshot| failed, for example, due to no disk space.
void HandleSaveChangesFailure(const SaveChangesSnapshot& snapshot);
- // Used by CheckTreeInvariants
+ // Used by CheckTreeInvariants.
void GetAllMetaHandles(BaseTransaction* trans, MetahandleSet* result);
+
+ // Used by VacuumAfterSaveChanges.
bool SafeToPurgeFromMemory(WriteTransaction* trans,
const EntryKernel* const entry) const;
-
// A helper used by GetTotalNodeCount.
void GetChildSetForKernel(
BaseTransaction*,
diff --git a/sync/syncable/directory_backing_store.cc b/sync/syncable/directory_backing_store.cc
index f87933b..9d18a2d 100644
--- a/sync/syncable/directory_backing_store.cc
+++ b/sync/syncable/directory_backing_store.cc
@@ -538,8 +538,8 @@ bool DirectoryBackingStore::RefreshColumns() {
return true;
}
-bool DirectoryBackingStore::LoadEntries(
- Directory::MetahandlesMap* handles_map) {
+bool DirectoryBackingStore::LoadEntries(Directory::MetahandlesMap* handles_map,
+ MetahandleSet* metahandles_to_purge) {
string select;
select.reserve(kUpdateStatementBufferSize);
select.append("SELECT ");
@@ -555,11 +555,25 @@ bool DirectoryBackingStore::LoadEntries(
return false;
int64 handle = kernel->ref(META_HANDLE);
- (*handles_map)[handle] = kernel.release();
+ if (SafeToPurgeOnLoading(*kernel))
+ metahandles_to_purge->insert(handle);
+ else
+ (*handles_map)[handle] = kernel.release();
}
return s.Succeeded();
}
+bool DirectoryBackingStore::SafeToPurgeOnLoading(
+ const EntryKernel& entry) const {
+ if (entry.ref(IS_DEL)) {
+ if (!entry.ref(IS_UNSYNCED) && !entry.ref(IS_UNAPPLIED_UPDATE))
+ return true;
+ else if (!entry.ref(ID).ServerKnows())
+ return true;
+ }
+ return false;
+}
+
bool DirectoryBackingStore::LoadDeleteJournals(
JournalIndex* delete_journals) {
string select;
@@ -643,21 +657,6 @@ bool DirectoryBackingStore::SaveEntryToDB(sql::Statement* save_statement,
return save_statement->Run();
}
-bool DirectoryBackingStore::DropDeletedEntries() {
- if (!db_->Execute("DELETE FROM metas "
- "WHERE is_del > 0 "
- "AND is_unsynced < 1 "
- "AND is_unapplied_update < 1")) {
- return false;
- }
- if (!db_->Execute("DELETE FROM metas "
- "WHERE is_del > 0 "
- "AND id LIKE 'c%'")) {
- return false;
- }
- return true;
-}
-
bool DirectoryBackingStore::SafeDropTable(const char* table_name) {
string query = "DROP TABLE IF EXISTS ";
query.append(table_name);
diff --git a/sync/syncable/directory_backing_store.h b/sync/syncable/directory_backing_store.h
index 254fff4..36a2f0a 100644
--- a/sync/syncable/directory_backing_store.h
+++ b/sync/syncable/directory_backing_store.h
@@ -49,16 +49,18 @@ class SYNC_EXPORT_PRIVATE DirectoryBackingStore : public base::NonThreadSafe {
virtual ~DirectoryBackingStore();
// Loads and drops all currently persisted meta entries into |handles_map|
- // and loads appropriate persisted kernel info into |info_bucket|.
+ // and loads appropriate persisted kernel info into |kernel_load_info|.
+ // The function determines which entries can be safely dropped and inserts
+ // their keys into |metahandles_to_purge|. It is up to the caller to
+ // perform the actual cleanup.
//
- // This function can perform some cleanup tasks behind the scenes. It will
- // clean up unused entries from the database and migrate to the latest
- // database version. The caller can safely ignore these details.
+ // This function will migrate to the latest database version.
//
// NOTE: On success (return value of OPENED), the buckets are populated with
// newly allocated items, meaning ownership is bestowed upon the caller.
virtual DirOpenResult Load(Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) = 0;
// Updates the on-disk store with the input |snapshot| as a database
@@ -90,16 +92,15 @@ class SYNC_EXPORT_PRIVATE DirectoryBackingStore : public base::NonThreadSafe {
bool CreateV75ModelsTable();
bool CreateV81ModelsTable();
- // We don't need to load any synced and applied deleted entries, we can
- // in fact just purge them forever on startup.
- bool DropDeletedEntries();
// Drops a table if it exists, harmless if the table did not already exist.
bool SafeDropTable(const char* table_name);
// Load helpers for entries and attributes.
- bool LoadEntries(Directory::MetahandlesMap* handles_map);
+ bool LoadEntries(Directory::MetahandlesMap* handles_map,
+ MetahandleSet* metahandles_to_purge);
bool LoadDeleteJournals(JournalIndex* delete_journals);
bool LoadInfo(Directory::KernelLoadInfo* info);
+ bool SafeToPurgeOnLoading(const EntryKernel& entry) const;
// Save/update helpers for entries. Return false if sqlite commit fails.
static bool SaveEntryToDB(sql::Statement* save_statement,
diff --git a/sync/syncable/directory_backing_store_unittest.cc b/sync/syncable/directory_backing_store_unittest.cc
index 8f679f1..7394b0d 100644
--- a/sync/syncable/directory_backing_store_unittest.cc
+++ b/sync/syncable/directory_backing_store_unittest.cc
@@ -46,10 +46,11 @@ class MigrationTest : public testing::TestWithParam<int> {
static bool LoadAndIgnoreReturnedData(DirectoryBackingStore *dbs) {
Directory::MetahandlesMap tmp_handles_map;
JournalIndex delete_journals;
+ MetahandleSet metahandles_to_purge;
STLValueDeleter<Directory::MetahandlesMap> deleter(&tmp_handles_map);
Directory::KernelLoadInfo kernel_load_info;
- return dbs->Load(&tmp_handles_map, &delete_journals, &kernel_load_info) ==
- OPENED;
+ return dbs->Load(&tmp_handles_map, &delete_journals, &metahandles_to_purge,
+ &kernel_load_info) == OPENED;
}
void SetUpVersion67Database(sql::Connection* connection);
@@ -3202,12 +3203,14 @@ TEST_F(DirectoryBackingStoreTest, MigrateVersion78To79) {
// Ensure the next_id has been incremented.
Directory::MetahandlesMap handles_map;
- JournalIndex delete_journals;;
+ JournalIndex delete_journals;
+ MetahandleSet metahandles_to_purge;
STLValueDeleter<Directory::MetahandlesMap> deleter(&handles_map);
Directory::KernelLoadInfo load_info;
s.Clear();
- ASSERT_TRUE(dbs->Load(&handles_map, &delete_journals, &load_info));
+ ASSERT_TRUE(dbs->Load(&handles_map, &delete_journals, &metahandles_to_purge,
+ &load_info));
EXPECT_LE(load_info.kernel_info.next_id, kInitialNextId - 65536);
}
@@ -3225,11 +3228,13 @@ TEST_F(DirectoryBackingStoreTest, MigrateVersion79To80) {
// Ensure the bag_of_chips has been set.
Directory::MetahandlesMap handles_map;
- JournalIndex delete_journals;;
+ JournalIndex delete_journals;
+ MetahandleSet metahandles_to_purge;
STLValueDeleter<Directory::MetahandlesMap> deleter(&handles_map);
Directory::KernelLoadInfo load_info;
- ASSERT_TRUE(dbs->Load(&handles_map, &delete_journals, &load_info));
+ ASSERT_TRUE(dbs->Load(&handles_map, &delete_journals, &metahandles_to_purge,
+ &load_info));
// Check that the initial value is the serialization of an empty ChipBag.
sync_pb::ChipBag chip_bag;
std::string serialized_chip_bag;
@@ -3438,11 +3443,13 @@ TEST_F(DirectoryBackingStoreTest, DetectInvalidPosition) {
// Trying to unpack this entry should signal that the DB is corrupted.
Directory::MetahandlesMap handles_map;
- JournalIndex delete_journals;;
+ JournalIndex delete_journals;
+ MetahandleSet metahandles_to_purge;
STLValueDeleter<Directory::MetahandlesMap> deleter(&handles_map);
Directory::KernelLoadInfo kernel_load_info;
ASSERT_EQ(FAILED_DATABASE_CORRUPT,
- dbs->Load(&handles_map, &delete_journals, &kernel_load_info));
+ dbs->Load(&handles_map, &delete_journals, &metahandles_to_purge,
+ &kernel_load_info));
}
TEST_P(MigrationTest, ToCurrentVersion) {
@@ -3529,13 +3536,17 @@ TEST_P(MigrationTest, ToCurrentVersion) {
syncable::Directory::KernelLoadInfo dir_info;
Directory::MetahandlesMap handles_map;
- JournalIndex delete_journals;;
+ JournalIndex delete_journals;
+ MetahandleSet metahandles_to_purge;
STLValueDeleter<Directory::MetahandlesMap> index_deleter(&handles_map);
{
scoped_ptr<TestDirectoryBackingStore> dbs(
new TestDirectoryBackingStore(GetUsername(), &connection));
- ASSERT_EQ(OPENED, dbs->Load(&handles_map, &delete_journals, &dir_info));
+ ASSERT_EQ(OPENED, dbs->Load(&handles_map, &delete_journals,
+ &metahandles_to_purge, &dir_info));
+ if (!metahandles_to_purge.empty())
+ dbs->DeleteEntries(metahandles_to_purge);
ASSERT_FALSE(dbs->needs_column_refresh_);
ASSERT_EQ(kCurrentDBVersion, dbs->GetVersion());
}
@@ -3903,20 +3914,22 @@ TEST_F(DirectoryBackingStoreTest, DeleteEntries) {
new TestDirectoryBackingStore(GetUsername(), &connection));
Directory::MetahandlesMap handles_map;
JournalIndex delete_journals;
+ MetahandleSet metahandles_to_purge;
Directory::KernelLoadInfo kernel_load_info;
STLValueDeleter<Directory::MetahandlesMap> index_deleter(&handles_map);
- dbs->Load(&handles_map, &delete_journals, &kernel_load_info);
+ dbs->Load(&handles_map, &delete_journals, &metahandles_to_purge,
+ &kernel_load_info);
size_t initial_size = handles_map.size();
ASSERT_LT(0U, initial_size) << "Test requires handles_map to delete.";
int64 first_to_die = handles_map.begin()->second->ref(META_HANDLE);
MetahandleSet to_delete;
to_delete.insert(first_to_die);
- EXPECT_TRUE(dbs->DeleteEntries(TestDirectoryBackingStore::METAS_TABLE,
- to_delete));
+ EXPECT_TRUE(dbs->DeleteEntries(to_delete));
STLDeleteValues(&handles_map);
- dbs->LoadEntries(&handles_map);
+ metahandles_to_purge.clear();
+ dbs->LoadEntries(&handles_map, &metahandles_to_purge);
EXPECT_EQ(initial_size - 1, handles_map.size());
bool delete_failed = false;
@@ -3935,11 +3948,11 @@ TEST_F(DirectoryBackingStoreTest, DeleteEntries) {
to_delete.insert(it->first);
}
- EXPECT_TRUE(dbs->DeleteEntries(TestDirectoryBackingStore::METAS_TABLE,
- to_delete));
+ EXPECT_TRUE(dbs->DeleteEntries(to_delete));
STLDeleteValues(&handles_map);
- dbs->LoadEntries(&handles_map);
+ metahandles_to_purge.clear();
+ dbs->LoadEntries(&handles_map, &metahandles_to_purge);
EXPECT_EQ(0U, handles_map.size());
}
diff --git a/sync/syncable/directory_unittest.cc b/sync/syncable/directory_unittest.cc
index 5f8cbed..aa974d5 100644
--- a/sync/syncable/directory_unittest.cc
+++ b/sync/syncable/directory_unittest.cc
@@ -552,6 +552,87 @@ TEST_F(SyncableDirectoryTest, ManageDeleteJournals) {
}
}
+TEST_F(SyncableDirectoryTest, TestPurgeDeletedEntriesOnReload) {
+ sync_pb::EntitySpecifics specifics;
+ AddDefaultFieldValue(PREFERENCES, &specifics);
+
+ const int kClientCount = 2;
+ const int kServerCount = 5;
+ const int kTestCount = kClientCount + kServerCount;
+ int64 handles[kTestCount];
+
+ // The idea is to recreate various combinations of IDs, IS_DEL,
+ // IS_UNSYNCED, and IS_UNAPPLIED_UPDATE flags to test all combinations
+ // for DirectoryBackingStore::SafeToPurgeOnLoading.
+ // 0: client ID, IS_DEL, IS_UNSYNCED
+ // 1: client ID, IS_UNSYNCED
+ // 2: server ID, IS_DEL, IS_UNSYNCED, IS_UNAPPLIED_UPDATE
+ // 3: server ID, IS_DEL, IS_UNSYNCED
+ // 4: server ID, IS_DEL, IS_UNAPPLIED_UPDATE
+ // 5: server ID, IS_DEL
+ // 6: server ID
+ {
+ WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
+
+ for (int i = 0; i < kTestCount; i++) {
+ std::string name = base::StringPrintf("item%d", i);
+ MutableEntry item(&trans, CREATE, PREFERENCES, trans.root_id(), name);
+ ASSERT_TRUE(item.good());
+
+ handles[i] = item.GetMetahandle();
+
+ if (i < kClientCount) {
+ item.PutId(TestIdFactory::FromNumber(i - kClientCount));
+ } else {
+ item.PutId(TestIdFactory::FromNumber(i));
+ }
+
+ item.PutUniqueClientTag(name);
+ item.PutIsUnsynced(true);
+ item.PutSpecifics(specifics);
+ item.PutServerSpecifics(specifics);
+
+ if (i >= kClientCount) {
+ item.PutBaseVersion(10);
+ item.PutServerVersion(10);
+ }
+
+ // Set flags
+ if (i != 1 && i != 6)
+ item.PutIsDel(true);
+
+ if (i >= 4)
+ item.PutIsUnsynced(false);
+
+ if (i == 2 || i == 4)
+ item.PutIsUnappliedUpdate(true);
+ }
+ }
+ ASSERT_EQ(OPENED, SimulateSaveAndReloadDir());
+
+ // Expect items 0 and 5 to be purged according to
+ // DirectoryBackingStore::SafeToPurgeOnLoading:
+ // - Item 0 is an item with IS_DEL flag and client ID.
+ // - Item 5 is an item with IS_DEL flag which has both
+ // IS_UNSYNCED and IS_UNAPPLIED_UPDATE unset.
+ std::vector<int64> expected_purged;
+ expected_purged.push_back(0);
+ expected_purged.push_back(5);
+
+ std::vector<int64> actually_purged;
+ {
+ ReadTransaction trans(FROM_HERE, dir().get());
+ for (int i = 0; i < kTestCount; i++) {
+ Entry item(&trans, GET_BY_HANDLE, handles[i]);
+ if (!item.good()) {
+ actually_purged.push_back(i);
+ }
+ }
+ }
+
+ EXPECT_EQ(expected_purged, actually_purged);
+}
+
TEST_F(SyncableDirectoryTest, TestBasicLookupNonExistantID) {
ReadTransaction rtrans(FROM_HERE, dir().get());
Entry e(&rtrans, GET_BY_ID, TestIdFactory::FromNumber(-99));
diff --git a/sync/syncable/in_memory_directory_backing_store.cc b/sync/syncable/in_memory_directory_backing_store.cc
index 57995f8..8f84baa 100644
--- a/sync/syncable/in_memory_directory_backing_store.cc
+++ b/sync/syncable/in_memory_directory_backing_store.cc
@@ -16,6 +16,7 @@ InMemoryDirectoryBackingStore::InMemoryDirectoryBackingStore(
DirOpenResult InMemoryDirectoryBackingStore::Load(
Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) {
if (!db_->is_open()) {
if (!db_->OpenInMemory())
@@ -32,9 +33,7 @@ DirOpenResult InMemoryDirectoryBackingStore::Load(
}
}
- if (!DropDeletedEntries())
- return FAILED_DATABASE_CORRUPT;
- if (!LoadEntries(handles_map))
+ if (!LoadEntries(handles_map, metahandles_to_purge))
return FAILED_DATABASE_CORRUPT;
if (!LoadDeleteJournals(delete_journals))
return FAILED_DATABASE_CORRUPT;
diff --git a/sync/syncable/in_memory_directory_backing_store.h b/sync/syncable/in_memory_directory_backing_store.h
index 0a0f9ba..d22e65f 100644
--- a/sync/syncable/in_memory_directory_backing_store.h
+++ b/sync/syncable/in_memory_directory_backing_store.h
@@ -26,6 +26,7 @@ class SYNC_EXPORT_PRIVATE InMemoryDirectoryBackingStore
explicit InMemoryDirectoryBackingStore(const std::string& dir_name);
DirOpenResult Load(Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) override;
void request_consistent_cache_guid() {
diff --git a/sync/syncable/invalid_directory_backing_store.cc b/sync/syncable/invalid_directory_backing_store.cc
index 6345521..1844d84 100644
--- a/sync/syncable/invalid_directory_backing_store.cc
+++ b/sync/syncable/invalid_directory_backing_store.cc
@@ -17,6 +17,7 @@ InvalidDirectoryBackingStore::~InvalidDirectoryBackingStore() {
DirOpenResult InvalidDirectoryBackingStore::Load(
Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) {
return FAILED_OPEN_DATABASE;
}
diff --git a/sync/syncable/invalid_directory_backing_store.h b/sync/syncable/invalid_directory_backing_store.h
index fd99653..2a6d065 100644
--- a/sync/syncable/invalid_directory_backing_store.h
+++ b/sync/syncable/invalid_directory_backing_store.h
@@ -19,6 +19,7 @@ class SYNC_EXPORT_PRIVATE InvalidDirectoryBackingStore
~InvalidDirectoryBackingStore() override;
DirOpenResult Load(Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) override;
private:
diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc
index 89c8b05..d627cf4 100644
--- a/sync/syncable/mutable_entry.cc
+++ b/sync/syncable/mutable_entry.cc
@@ -184,9 +184,9 @@ void MutableEntry::PutIsDel(bool value) {
// - Ensure that the item is never committed to the server.
// - Allow any items with the same UNIQUE_CLIENT_TAG created on other
// clients to override this entry.
- // - Let us delete this entry permanently through
- // DirectoryBackingStore::DropDeletedEntries() when we next restart sync.
- // This will save memory and avoid crbug.com/125381.
+ // - Let us delete this entry permanently when we next restart sync - see
+ // DirectoryBackingStore::SafeToPurgeOnLoading.
+ // This will avoid crbug.com/125381.
// Note: do not unset IsUnsynced if the syncer is in the middle of
// attempting to commit this entity.
if (!GetId().ServerKnows() && !GetSyncing()) {
diff --git a/sync/syncable/on_disk_directory_backing_store.cc b/sync/syncable/on_disk_directory_backing_store.cc
index 1bebf9a..dcbaedd 100644
--- a/sync/syncable/on_disk_directory_backing_store.cc
+++ b/sync/syncable/on_disk_directory_backing_store.cc
@@ -37,6 +37,7 @@ OnDiskDirectoryBackingStore::~OnDiskDirectoryBackingStore() { }
DirOpenResult OnDiskDirectoryBackingStore::TryLoad(
Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) {
DCHECK(CalledOnValidThread());
if (!db_->is_open()) {
@@ -47,9 +48,7 @@ DirOpenResult OnDiskDirectoryBackingStore::TryLoad(
if (!InitializeTables())
return FAILED_OPEN_DATABASE;
- if (!DropDeletedEntries())
- return FAILED_DATABASE_CORRUPT;
- if (!LoadEntries(handles_map))
+ if (!LoadEntries(handles_map, metahandles_to_purge))
return FAILED_DATABASE_CORRUPT;
if (!LoadDeleteJournals(delete_journals))
return FAILED_DATABASE_CORRUPT;
@@ -65,9 +64,10 @@ DirOpenResult OnDiskDirectoryBackingStore::TryLoad(
DirOpenResult OnDiskDirectoryBackingStore::Load(
Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) {
DirOpenResult result = TryLoad(handles_map, delete_journals,
- kernel_load_info);
+ metahandles_to_purge, kernel_load_info);
if (result == OPENED) {
UMA_HISTOGRAM_ENUMERATION(
"Sync.DirectoryOpenResult", FIRST_TRY_SUCCESS, RESULT_COUNT);
@@ -89,7 +89,8 @@ DirOpenResult OnDiskDirectoryBackingStore::Load(
db_->set_histogram_tag("SyncDirectory");
base::DeleteFile(backing_filepath_, false);
- result = TryLoad(handles_map, delete_journals, kernel_load_info);
+ result = TryLoad(handles_map, delete_journals, metahandles_to_purge,
+ kernel_load_info);
if (result == OPENED) {
UMA_HISTOGRAM_ENUMERATION(
"Sync.DirectoryOpenResult", SECOND_TRY_SUCCESS, RESULT_COUNT);
diff --git a/sync/syncable/on_disk_directory_backing_store.h b/sync/syncable/on_disk_directory_backing_store.h
index fdeda4d..43bc74d 100644
--- a/sync/syncable/on_disk_directory_backing_store.h
+++ b/sync/syncable/on_disk_directory_backing_store.h
@@ -22,14 +22,15 @@ class SYNC_EXPORT_PRIVATE OnDiskDirectoryBackingStore
~OnDiskDirectoryBackingStore() override;
DirOpenResult Load(Directory::MetahandlesMap* handles_map,
JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
Directory::KernelLoadInfo* kernel_load_info) override;
// A helper function that will make one attempt to load the directory.
// Unlike Load(), it does not attempt to recover from failure.
- DirOpenResult TryLoad(
- Directory::MetahandlesMap* handles_map,
- JournalIndex* delete_journals,
- Directory::KernelLoadInfo* kernel_load_info);
+ DirOpenResult TryLoad(Directory::MetahandlesMap* handles_map,
+ JournalIndex* delete_journals,
+ MetahandleSet* metahandles_to_purge,
+ Directory::KernelLoadInfo* kernel_load_info);
protected:
// Subclasses may override this to avoid a possible DCHECK.