Skip to content

Commit

Permalink
Split the limit on IGs by owner for regular/negative IGs.
Browse files Browse the repository at this point in the history 
We currently enforce a limit on the number of interest groups by owner,
which is enforced at DB maintenance time. With the introduction of
negative targeting interest groups, this limit initially applied to the
combined number of regular and negative interest groups. This change
splits that limit into two separate limits: one on regular interest
groups, and a different limit on the number of negative interest groups.
The negative interest group limit is intentionally set much higher
because negative interest groups are much smaller and have a much lower
impact on auction performance. The limit currently enforced on the total
size of all interest groups by owner remains a combined limit on all of
an owner's regular and negative interest groups.

This matches the proposal posted at
WICG/turtledove#798 (comment).

Change-Id: I4886e142587404be49767cd61cebf85767061137
Bug: 1464874
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4902233
Reviewed-by: Russ Hamilton <behamilton@google.com>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Orr Bernstein <orrb@google.com>
Cr-Commit-Position: refs/heads/main@{#1203584}
  • Loading branch information
@orrb1
orrb1 authored and Chromium LUCI CQ committed Sep 30, 2023
1 parent bf52068 commit 6c1a69b
Show file tree
Hide file tree
Showing 5 changed files with 237 additions and 35 deletions.
185 changes: 154 additions & 31 deletions content/browser/interest_group/interest_group_storage.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ const base::FilePath::CharType kDatabasePath[] =
// Version 15 - 2023/08 - crrev.com/c/4808727
// Version 16 - 2023/08 - crrev.com/c/4822944
// Version 17 - 2023/09 - crrev.com/c/4852051
// Version 18 - 2023/09 - crrev.com/c/4902233
//
// Version 1 adds a table for interest groups.
// Version 2 adds a column for rate limiting interest group updates.
Expand All @@ -97,7 +98,9 @@ const base::FilePath::CharType kDatabasePath[] =
// table to protobuf format.
// Version 17 adds interest group name and owner columns to the k-anonymity
// table.
const int kCurrentVersionNumber = 17;
// Version 18 adds a new index on IG type (regular vs negative) to support
// split caps on max interest groups per owner.
const int kCurrentVersionNumber = 18;

// Earliest version of the code which can use a |kCurrentVersionNumber|
// database without failing.
Expand Down Expand Up @@ -555,7 +558,7 @@ void MergePrioritySignalsOverrides(

// Adds indices to the `interest_group` table. Called after the table has been
// created.
bool CreateInterestGroupIndices(sql::Database& db) {
bool CreateInterestGroupIndicesForV17AndPrior(sql::Database& db) {
// Index on group expiration. Owner and Name are only here to speed up
// queries that don't need the full group.
DCHECK(!db.DoesIndexExist("interest_group_expiration"));
Expand Down Expand Up @@ -594,6 +597,23 @@ bool CreateInterestGroupIndices(sql::Database& db) {
return true;
}

bool CreateInterestGroupIndicesForV18(sql::Database& db) {
CreateInterestGroupIndicesForV17AndPrior(db);

// Index on group expiration by owner and IG type (regular vs negative)
DCHECK(!db.DoesIndexExist("interest_group_owner_and_type"));
static const char kInterestGroupOwnerAndTypeIndexSql[] =
// clang-format off
"CREATE INDEX interest_group_owner_and_type"
" ON interest_groups("
"LENGTH(additional_bid_key) == 0,owner,expiration DESC,name)";
// clang-format on
if (!db.Execute(kInterestGroupOwnerAndTypeIndexSql)) {
return false;
}
return true;
}

// Create indices on the k_anon table.
bool CreateKAnonIndices(sql::Database& db) {
DCHECK(!db.DoesIndexExist("kanon_key_idx"));
Expand Down Expand Up @@ -721,7 +741,7 @@ bool InsertKAnonForJoinedInterestGroup(sql::Database& db,

// Initializes the tables, returning true on success.
// The tables cannot exist when calling this function.
bool CreateV17Schema(sql::Database& db) {
bool CreateV18Schema(sql::Database& db) {
// IMPORTANT: If you add or remove fields, you need to update
// `ClearExcessiveStorage()` to consider the size of added/removed fields for
// storage usage calculations.
Expand Down Expand Up @@ -766,7 +786,7 @@ bool CreateV17Schema(sql::Database& db) {
return false;
}

if (!CreateInterestGroupIndices(db)) {
if (!CreateInterestGroupIndicesForV18(db)) {
return false;
}

Expand Down Expand Up @@ -849,6 +869,20 @@ bool CreateV17Schema(sql::Database& db) {
return true;
}

bool UpgradeV17SchemaToV18(sql::Database& db, sql::MetaTable& meta_table) {
DCHECK(!db.DoesIndexExist("interest_group_owner_and_type"));
static const char kInterestGroupOwnerAndTypeIndexSql[] =
// clang-format off
"CREATE INDEX interest_group_owner_and_type"
" ON interest_groups("
"LENGTH(additional_bid_key) == 0,owner,expiration DESC,name)";
// clang-format on
if (!db.Execute(kInterestGroupOwnerAndTypeIndexSql)) {
return false;
}
return true;
}

bool UpgradeV16SchemaToV17(sql::Database& db, sql::MetaTable& meta_table) {
static const char kCreateKAnonTableSql[] =
"CREATE TABLE k_anon_new("
Expand Down Expand Up @@ -1051,7 +1085,7 @@ bool UpgradeV15SchemaToV16(sql::Database& db, sql::MetaTable& meta_table) {
return false;
}

return CreateInterestGroupIndices(db);
return CreateInterestGroupIndicesForV17AndPrior(db);
}

bool UpgradeV14SchemaToV15(sql::Database& db, sql::MetaTable& meta_table) {
Expand Down Expand Up @@ -1141,7 +1175,7 @@ bool UpgradeV14SchemaToV15(sql::Database& db, sql::MetaTable& meta_table) {
return false;
}

return CreateInterestGroupIndices(db);
return CreateInterestGroupIndicesForV17AndPrior(db);
}

bool UpgradeV13SchemaToV14(sql::Database& db, sql::MetaTable& meta_table) {
Expand Down Expand Up @@ -1229,7 +1263,7 @@ bool UpgradeV13SchemaToV14(sql::Database& db, sql::MetaTable& meta_table) {
return false;
}

return CreateInterestGroupIndices(db);
return CreateInterestGroupIndicesForV17AndPrior(db);
}

bool UpgradeV12SchemaToV13(sql::Database& db, sql::MetaTable& meta_table) {
Expand Down Expand Up @@ -1315,7 +1349,7 @@ bool UpgradeV12SchemaToV13(sql::Database& db, sql::MetaTable& meta_table) {
return false;
}

return CreateInterestGroupIndices(db);
return CreateInterestGroupIndicesForV17AndPrior(db);
}

bool UpgradeV11SchemaToV12(sql::Database& db, sql::MetaTable& meta_table) {
Expand Down Expand Up @@ -1397,7 +1431,7 @@ bool UpgradeV11SchemaToV12(sql::Database& db, sql::MetaTable& meta_table) {
return false;
}

return CreateInterestGroupIndices(db);
return CreateInterestGroupIndicesForV17AndPrior(db);
}

bool UpgradeV10SchemaToV11(sql::Database& db, sql::MetaTable& meta_table) {
Expand Down Expand Up @@ -1475,7 +1509,7 @@ bool UpgradeV10SchemaToV11(sql::Database& db, sql::MetaTable& meta_table) {
return false;
}

return CreateInterestGroupIndices(db);
return CreateInterestGroupIndicesForV17AndPrior(db);
}

bool UpgradeV9SchemaToV10(sql::Database& db, sql::MetaTable& meta_table) {
Expand Down Expand Up @@ -2739,6 +2773,66 @@ absl::optional<std::vector<std::string>> DoGetInterestGroupNamesForOwner(
return result;
}

absl::optional<std::vector<std::string>>
DoGetAllRegularInterestGroupNamesForOwner(sql::Database& db,
const url::Origin& owner) {
// clang-format off
sql::Statement get_names(
db.GetCachedStatement(SQL_FROM_HERE,
"SELECT name "
"FROM interest_groups "
"WHERE LENGTH(additional_bid_key) == 0 AND owner=? "
"ORDER BY expiration DESC"));
// clang-format on

if (!get_names.is_valid()) {
return absl::nullopt;
}

get_names.Reset(true);
get_names.BindString(0, Serialize(owner));

std::vector<std::string> result;
while (get_names.Step()) {
result.push_back(get_names.ColumnString(0));
}
if (!get_names.Succeeded()) {
return absl::nullopt;
}

return result;
}

absl::optional<std::vector<std::string>>
DoGetAllNegativeInterestGroupNamesForOwner(sql::Database& db,
const url::Origin& owner) {
// clang-format off
sql::Statement get_names(
db.GetCachedStatement(SQL_FROM_HERE,
"SELECT name "
"FROM interest_groups "
"WHERE NOT LENGTH(additional_bid_key) == 0 AND owner=? "
"ORDER BY expiration DESC"));
// clang-format on

if (!get_names.is_valid()) {
return absl::nullopt;
}

get_names.Reset(true);
get_names.BindString(0, Serialize(owner));

std::vector<std::string> result;
while (get_names.Step()) {
result.push_back(get_names.ColumnString(0));
}
if (!get_names.Succeeded()) {
return absl::nullopt;
}

return result;
}

absl::optional<std::vector<StorageInterestGroup::KAnonymityData>>
DoGetKAnonymityData(sql::Database& db,
const blink::InterestGroupKey& group_key) {
Expand Down Expand Up @@ -3092,9 +3186,30 @@ bool DeleteOldWins(sql::Database& db, base::Time cutoff) {
return true;
}

bool DoClearExcessInterestGroups(
sql::Database& db,
const url::Origin& affected_origin,
const absl::optional<std::vector<std::string>> maybe_interest_groups,
size_t max_owner_interest_groups) {
if (!maybe_interest_groups) {
return false;
}
for (size_t group_idx = max_owner_interest_groups;
group_idx < maybe_interest_groups.value().size(); group_idx++) {
if (!DoRemoveInterestGroup(
db,
blink::InterestGroupKey(
affected_origin, maybe_interest_groups.value()[group_idx]))) {
return false;
}
}
return true;
}

bool ClearExcessInterestGroups(sql::Database& db,
size_t max_owners,
size_t max_owner_interest_groups) {
size_t max_owner_regular_interest_groups,
size_t max_owner_negative_interest_groups) {
const base::Time distant_past = base::Time::Min();
const absl::optional<std::vector<url::Origin>> maybe_all_origins =
DoGetAllInterestGroupOwners(db, distant_past);
Expand All @@ -3104,23 +3219,17 @@ bool ClearExcessInterestGroups(sql::Database& db,
for (size_t owner_idx = 0; owner_idx < maybe_all_origins.value().size();
owner_idx++) {
const url::Origin& affected_origin = maybe_all_origins.value()[owner_idx];
const absl::optional<std::vector<std::string>> maybe_interest_groups =
DoGetInterestGroupNamesForOwner(db, affected_origin, distant_past);
if (!maybe_interest_groups) {
if (!DoClearExcessInterestGroups(
db, affected_origin,
DoGetAllRegularInterestGroupNamesForOwner(db, affected_origin),
owner_idx < max_owners ? max_owner_regular_interest_groups : 0)) {
return false;
}
size_t first_idx = max_owner_interest_groups;
if (owner_idx >= max_owners) {
first_idx = 0;
}
for (size_t group_idx = first_idx;
group_idx < maybe_interest_groups.value().size(); group_idx++) {
if (!DoRemoveInterestGroup(
db,
blink::InterestGroupKey(
affected_origin, maybe_interest_groups.value()[group_idx]))) {
return false;
}
if (!DoClearExcessInterestGroups(
db, affected_origin,
DoGetAllNegativeInterestGroupNamesForOwner(db, affected_origin),
owner_idx < max_owners ? max_owner_negative_interest_groups : 0)) {
return false;
}
}
return true;
Expand Down Expand Up @@ -3272,13 +3381,16 @@ bool DoPerformDatabaseMaintenance(sql::Database& db,
base::Time now,
size_t max_owners,
size_t max_owner_storage_size,
size_t max_owner_interest_groups) {
size_t max_owner_regular_interest_groups,
size_t max_owner_negative_interest_groups) {
SCOPED_UMA_HISTOGRAM_TIMER_MICROS("Storage.InterestGroup.DBMaintenanceTime");
sql::Transaction transaction(&db);
if (!transaction.Begin()) {
return false;
}
if (!ClearExcessInterestGroups(db, max_owners, max_owner_interest_groups)) {
if (!ClearExcessInterestGroups(db, max_owners,
max_owner_regular_interest_groups,
max_owner_negative_interest_groups)) {
return false;
}
if (!ClearExpiredInterestGroups(db, now)) {
Expand Down Expand Up @@ -3321,8 +3433,11 @@ constexpr base::TimeDelta InterestGroupStorage::kUpdateFailedBackoffPeriod;
InterestGroupStorage::InterestGroupStorage(const base::FilePath& path)
: path_to_database_(DBPath(path)),
max_owners_(blink::features::kInterestGroupStorageMaxOwners.Get()),
max_owner_interest_groups_(
max_owner_regular_interest_groups_(
blink::features::kInterestGroupStorageMaxGroupsPerOwner.Get()),
max_owner_negative_interest_groups_(
blink::features::kInterestGroupStorageMaxNegativeGroupsPerOwner
.Get()),
max_owner_storage_size_(
blink::features::kInterestGroupStorageMaxStoragePerOwner.Get()),
max_ops_before_maintenance_(
Expand Down Expand Up @@ -3428,7 +3543,7 @@ bool InterestGroupStorage::InitializeSchema() {
}

if (new_db) {
return CreateV17Schema(*db_);
return CreateV18Schema(*db_);
}

const int db_version = meta_table.GetVersionNumber();
Expand Down Expand Up @@ -3504,6 +3619,11 @@ bool InterestGroupStorage::InitializeSchema() {
if (!UpgradeV16SchemaToV17(*db_, meta_table)) {
return false;
}
ABSL_FALLTHROUGH_INTENDED;
case 17:
if (!UpgradeV17SchemaToV18(*db_, meta_table)) {
return false;
}

if (!meta_table.SetVersionNumber(kCurrentVersionNumber)) {
return false;
Expand Down Expand Up @@ -3870,7 +3990,10 @@ void InterestGroupStorage::PerformDBMaintenance() {
DoPerformDatabaseMaintenance(
*db_, last_maintenance_time_, /*max_owners=*/max_owners_,
/*max_owner_storage_size=*/max_owner_storage_size_,
/*max_owner_interest_groups=*/max_owner_interest_groups_);
/*max_owner_regular_interest_groups=*/
max_owner_regular_interest_groups_,
/*max_owner_negative_interest_groups=*/
max_owner_negative_interest_groups_);
}
}

Expand Down
9 changes: 6 additions & 3 deletions content/browser/interest_group/interest_group_storage.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,13 @@ class CONTENT_EXPORT InterestGroupStorage {
const base::FilePath path_to_database_;
// Maximum number of interest groups, or interest group owners to keep in the
// database.
// Set by the related blink::feature parameters kInterestGroupStorageMaxOwners
// and kInterestGroupStorageMaxGroupsPerOwner.
// Set by the related blink::feature parameters
// kInterestGroupStorageMaxOwners,
// kInterestGroupStorageMaxGroupsPerOwner, and
// kInterestGroupStorageMaxNegativeGroupsPerOwner.
const size_t max_owners_;
const size_t max_owner_interest_groups_;
const size_t max_owner_regular_interest_groups_;
const size_t max_owner_negative_interest_groups_;
const size_t max_owner_storage_size_;

// Maximum number of operations allowed between maintenance calls.
Expand Down

0 comments on commit 6c1a69b

Please sign in to comment.