Skip to content

Commit

Permalink
Deprecate the unused MAX_ITEM_COUNT GC directive
Browse files Browse the repository at this point in the history 
Bug: 881286
Change-Id: I248f02f371a5a8c3bddf3dafffe825141300330b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1583811
Commit-Queue: Victor Vianna <victorvianna@google.com>
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Auto-Submit: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#654041}
  • Loading branch information
Victor Hugo Vianna Silva authored and Commit Bot committed Apr 25, 2019
1 parent cffbce7 commit 6552a9d
Show file tree
Hide file tree
Showing 8 changed files with 3 additions and 245 deletions.
6 changes: 0 additions & 6 deletions components/sync/engine_impl/directory_update_handler.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,12 +309,6 @@ void DirectoryUpdateHandler::ExpireEntriesIfNeeded(
cached_gc_directive_aged_out_day_ = to_be_expired;
}
}

if (new_gc_directive.has_max_number_of_items()) {
DCHECK(new_gc_directive.max_number_of_items());
ExpireEntriesByItemLimit(dir_, trans, type_,
new_gc_directive.max_number_of_items());
}
}

} // namespace syncer
71 changes: 0 additions & 71 deletions components/sync/engine_impl/directory_update_handler_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,77 +370,6 @@ TEST_F(DirectoryUpdateHandlerProcessUpdateTest, GarbageCollectionByAge) {
EXPECT_TRUE(EntryExists(e2->id_string()));
}

// Create 3 entries, one is 15-days-old, one is 10-days-old, another is
// 5-days-old. Check if sync will delete 15-days-old entry when server set
// max_number_of_items is 2.
TEST_F(DirectoryUpdateHandlerProcessUpdateTest, GarbageCollectionByItemLimit) {
DirectoryTypeDebugInfoEmitter emitter(DEPRECATED_SYNCED_NOTIFICATIONS,
&type_observers_);
DirectoryUpdateHandler handler(dir(), DEPRECATED_SYNCED_NOTIFICATIONS,
ui_worker(), &emitter);
StatusController status;

sync_pb::DataTypeProgressMarker progress;
progress.set_data_type_id(
GetSpecificsFieldNumberFromModelType(DEPRECATED_SYNCED_NOTIFICATIONS));
progress.set_token("token");
progress.mutable_gc_directive()->set_max_number_of_items(3);

sync_pb::DataTypeContext context;
context.set_data_type_id(
GetSpecificsFieldNumberFromModelType(DEPRECATED_SYNCED_NOTIFICATIONS));
context.set_context("context");
context.set_version(1);

std::unique_ptr<sync_pb::SyncEntity> e1 =
CreateUpdate(SyncableIdToProto(Id::CreateFromServerId("e1")), "",
DEPRECATED_SYNCED_NOTIFICATIONS);
e1->set_mtime(
TimeToProtoTime(base::Time::Now() - base::TimeDelta::FromDays(15)));

std::unique_ptr<sync_pb::SyncEntity> e2 =
CreateUpdate(SyncableIdToProto(Id::CreateFromServerId("e2")), "",
DEPRECATED_SYNCED_NOTIFICATIONS);
e2->set_mtime(
TimeToProtoTime(base::Time::Now() - base::TimeDelta::FromDays(5)));

std::unique_ptr<sync_pb::SyncEntity> e3 =
CreateUpdate(SyncableIdToProto(Id::CreateFromServerId("e3")), "",
DEPRECATED_SYNCED_NOTIFICATIONS);
e3->set_mtime(
TimeToProtoTime(base::Time::Now() - base::TimeDelta::FromDays(10)));

// Add to the applicable updates list.
SyncEntityList updates;
updates.push_back(e1.get());
updates.push_back(e2.get());
updates.push_back(e3.get());

// Process and apply updates.
EXPECT_EQ(
SyncerError::SYNCER_OK,
handler.ProcessGetUpdatesResponse(progress, context, updates, &status)
.value());
handler.ApplyUpdates(&status);

// Verify none is deleted because they are unapplied during GC.
EXPECT_TRUE(TypeRootExists(DEPRECATED_SYNCED_NOTIFICATIONS));
EXPECT_TRUE(EntryExists(e1->id_string()));
EXPECT_TRUE(EntryExists(e2->id_string()));

// Process and apply again. 15-days-old entry is deleted.
progress.mutable_gc_directive()->set_max_number_of_items(2);
EXPECT_EQ(SyncerError::SYNCER_OK,
handler
.ProcessGetUpdatesResponse(progress, context, SyncEntityList(),
&status)
.value());
handler.ApplyUpdates(&status);
EXPECT_FALSE(EntryExists(e1->id_string()));
EXPECT_TRUE(EntryExists(e2->id_string()));
EXPECT_TRUE(EntryExists(e3->id_string()));
}

TEST_F(DirectoryUpdateHandlerProcessUpdateTest, ContextVersion) {
DirectoryTypeDebugInfoEmitter emitter(DEPRECATED_SYNCED_NOTIFICATIONS,
&type_observers_);
Expand Down
56 changes: 0 additions & 56 deletions components/sync/engine_impl/process_updates_util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,24 +273,6 @@ void ProcessUpdate(const sync_pb::SyncEntity& update,
return;
}

bool CompareTimes(const base::Time& left, const base::Time& right) {
return left > right;
}

// This function use quick select algorithm (std::nth_element) to find the |n|th
// bigest number in the vector |times|.
base::Time FindTheNthBigestTime(std::vector<base::Time> times, size_t n) {
DCHECK(n);

if (n > times.size())
return base::Time::UnixEpoch();

std::nth_element(times.begin(), times.begin() + n - 1, times.end(),
&CompareTimes);

return times[n - 1];
}

} // namespace

void ProcessDownloadedUpdates(syncable::Directory* dir,
Expand Down Expand Up @@ -369,42 +351,4 @@ void ExpireEntriesByAge(syncable::Directory* dir,
}
}

void ExpireEntriesByItemLimit(syncable::Directory* dir,
syncable::ModelNeutralWriteTransaction* trans,
ModelType type,
int64_t max_number_of_items) {
syncable::Directory::Metahandles handles;
dir->GetMetaHandlesOfType(trans, type, &handles);

size_t limited_number = max_number_of_items;
if (limited_number >= handles.size())
return;

std::vector<base::Time> all_times;
for (size_t i = 0; i < handles.size(); ++i) {
syncable::ModelNeutralMutableEntry entry(trans, syncable::GET_BY_HANDLE,
handles[i]);
all_times.push_back(entry.GetMtime());
}
base::Time expired_time =
FindTheNthBigestTime(std::move(all_times), limited_number);

for (size_t i = 0; i < handles.size(); ++i) {
syncable::ModelNeutralMutableEntry entry(trans, syncable::GET_BY_HANDLE,
handles[i]);
if (!entry.good() || !entry.GetId().ServerKnows() ||
entry.GetUniqueServerTag() == ModelTypeToRootTag(type) ||
entry.GetIsUnappliedUpdate() || entry.GetIsUnsynced() ||
entry.GetIsDel() || entry.GetServerIsDel() ||
entry.GetMtime() >= expired_time) {
continue;
}

// Mark entry as unapplied update first to ensure journaling the deletion.
entry.PutIsUnappliedUpdate(true);
// Mark entry as deleted by server.
entry.PutServerIsDel(true);
}
}

} // namespace syncer
6 changes: 0 additions & 6 deletions components/sync/engine_impl/process_updates_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ void ExpireEntriesByAge(syncable::Directory* dir,
ModelType type,
int32_t age_watermark_in_days);

// If the number of sync entities exceeds |max_number_of_items|, sync
// client will tombstone the extra sync entities based on the LRU rule.
void ExpireEntriesByItemLimit(syncable::Directory* dir,
syncable::ModelNeutralWriteTransaction* trans,
ModelType type,
int64_t max_number_of_items);

} // namespace syncer

Expand Down
55 changes: 0 additions & 55 deletions components/sync/model_impl/client_tag_based_model_type_processor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,6 @@ namespace syncer {

namespace {

bool CompareProtoTimeStamp(const int64_t left, const int64_t right) {
return left > right;
}

// This function use quick select algorithm (std::nth_element) to find the |n|th
// bigest number in the vector |time_stamps|.
int64_t FindTheNthBigestProtoTimeStamp(std::vector<int64_t> time_stamps,
size_t n) {
DCHECK(n);

if (n > time_stamps.size())
return 0;

std::nth_element(time_stamps.begin(), time_stamps.begin() + n - 1,
time_stamps.end(), &CompareProtoTimeStamp);

return time_stamps[n - 1];
}

int CountNonTombstoneEntries(
const std::map<std::string, std::unique_ptr<ProcessorEntity>>& entities) {
int count = 0;
Expand Down Expand Up @@ -1365,13 +1346,6 @@ void ClientTagBasedModelTypeProcessor::ExpireEntriesIfNeeded(
}
}

if (new_gc_directive.has_max_number_of_items()) {
DCHECK(new_gc_directive.max_number_of_items());
ExpireEntriesByItemLimit(new_gc_directive.max_number_of_items(),
metadata_changes.get());
has_expired_changes = true;
}

if (has_expired_changes)
bridge_->ApplySyncChanges(std::move(metadata_changes), EntityChangeList());
}
Expand Down Expand Up @@ -1423,35 +1397,6 @@ void ClientTagBasedModelTypeProcessor::ExpireEntriesByAge(
ClearMetadataForEntries(storage_key_to_be_deleted, metadata_changes);
}

void ClientTagBasedModelTypeProcessor::ExpireEntriesByItemLimit(
int32_t max_number_of_items,
MetadataChangeList* metadata_changes) {
DCHECK(metadata_changes);

size_t limited_number = max_number_of_items;
if (limited_number >= entities_.size())
return;

std::vector<int64_t> all_proto_times;
for (const auto& kv : entities_) {
ProcessorEntity* entity = kv.second.get();
all_proto_times.push_back(entity->metadata().modification_time());
}
int64_t expired_proto_time = FindTheNthBigestProtoTimeStamp(
std::move(all_proto_times), limited_number);

std::vector<std::string> storage_key_to_be_deleted;
for (const auto& kv : entities_) {
ProcessorEntity* entity = kv.second.get();
if (!entity->IsUnsynced() &&
entity->metadata().modification_time() < expired_proto_time) {
storage_key_to_be_deleted.push_back(entity->storage_key());
}
}

ClearMetadataForEntries(storage_key_to_be_deleted, metadata_changes);
}

void ClientTagBasedModelTypeProcessor::RemoveEntity(
ProcessorEntity* entity,
MetadataChangeList* metadata_change_list) {
Expand Down
8 changes: 0 additions & 8 deletions components/sync/model_impl/client_tag_based_model_type_processor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,6 @@ class ClientTagBasedModelTypeProcessor : public ModelTypeProcessor,
void ExpireEntriesByAge(int32_t age_watermark_in_days,
MetadataChangeList* metadata_changes);

// If the number of |entities_| exceeds |max_number_of_items|, the
// processor removes metadata for the extra sync entities based on the LRU
// rule.
// This is used to limit the amount of data stored in sync, and this does not
// tell the bridge to delete the actual data.
void ExpireEntriesByItemLimit(int32_t max_number_of_items,
MetadataChangeList* metadata_changes);

// Removes |entity| and clears metadata for |entity| from
// |metadata_change_list|.
void RemoveEntity(ProcessorEntity* entity,
Expand Down
41 changes: 0 additions & 41 deletions components/sync/model_impl/client_tag_based_model_type_processor_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2213,47 +2213,6 @@ TEST_F(ClientTagBasedModelTypeProcessorTest,
EXPECT_EQ(0U, worker()->GetNumPendingCommits());
}

// Tests that ClientTagBasedModelTypeProcessor can do garbage collection by item
// limit. Create 3 entries, one is 15-days-old, one is 10-days-old, another is
// 5-days-old. Check if sync will delete 15-days-old entry when server set
// limited item is 2 days.
TEST_F(ClientTagBasedModelTypeProcessorTest,
ShouldApplyGarbageCollectionByItemLimit) {
InitializeToReadyState();

// Create 3 entries, one is 15-days-old, one is 10-days-old, another is
// 5-days-old.
std::unique_ptr<EntityData> entity_data =
bridge()->GenerateEntityData(kKey1, kValue1);
entity_data->modification_time =
base::Time::Now() - base::TimeDelta::FromDays(15);
WriteItemAndAck(kKey1, std::move(entity_data));
entity_data = bridge()->GenerateEntityData(kKey2, kValue2);
entity_data->modification_time =
base::Time::Now() - base::TimeDelta::FromDays(5);
WriteItemAndAck(kKey2, std::move(entity_data));
entity_data = bridge()->GenerateEntityData(kKey3, kValue3);
entity_data->modification_time =
base::Time::Now() - base::TimeDelta::FromDays(10);
WriteItemAndAck(kKey3, std::move(entity_data));

// Verify entries are created correctly.
EXPECT_EQ(3U, ProcessorEntityCount());
EXPECT_EQ(3U, db()->metadata_count());
EXPECT_EQ(3U, db()->data_count());
EXPECT_EQ(0U, worker()->GetNumPendingCommits());

// Expired the entries which are older than 10 days.
sync_pb::GarbageCollectionDirective garbage_collection_directive;
garbage_collection_directive.set_max_number_of_items(2);
worker()->UpdateWithGarbageCollection(garbage_collection_directive);

EXPECT_EQ(2U, ProcessorEntityCount());
EXPECT_EQ(2U, db()->metadata_count());
EXPECT_EQ(3U, db()->data_count());
EXPECT_EQ(0U, worker()->GetNumPendingCommits());
}

TEST_F(ClientTagBasedModelTypeProcessorTest,
ShouldDeleteMetadataWhenCacheGuidMismatch) {
// Commit item.
Expand Down
5 changes: 3 additions & 2 deletions components/sync/protocol/sync.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ message GarbageCollectionDirective {
UNKNOWN = 0;
VERSION_WATERMARK = 1;
AGE_WATERMARK = 2;
MAX_ITEM_COUNT = 3;
DEPRECATED_MAX_ITEM_COUNT = 3 [deprecated = true];
}

optional Type type = 1 [default = UNKNOWN];
Expand All @@ -561,7 +561,8 @@ message GarbageCollectionDirective {
// This field specifies the max number of items that the client should keep
// for a specific datatype. If the number of items exceeds this limit, the
// client should purge the extra sync entities based on the LRU rule.
optional int32 max_number_of_items = 4;
// Deprecated in M76.
optional int32 deprecated_max_number_of_items = 4 [deprecated = true];
}

message DataTypeProgressMarker {
Expand Down

0 comments on commit 6552a9d

Please sign in to comment.