Remove unused subgroup cache
Subgroups are indirectly cached by the group cache. All former users of the subgroup cache now directly access the subgroups of a loaded group. Hence, there is no need to keep the subgroup cache anymore. Change-Id: Iaa019b5cac87576eaaa0f7bce71dc91fa8f3c775
This commit is contained in:

committed by
Dave Borowitz

parent
f74e3d7d70
commit
8238f71b92
@@ -66,7 +66,6 @@ Intended for interactive use only.
|
|||||||
groups_bysubgroup | 230 | 2.4ms | 62% |
|
groups_bysubgroup | 230 | 2.4ms | 62% |
|
||||||
groups_byuuid | 5612 | 29.2ms | 99% |
|
groups_byuuid | 5612 | 29.2ms | 99% |
|
||||||
groups_external | 1 | 1.5s | 98% |
|
groups_external | 1 | 1.5s | 98% |
|
||||||
groups_subgroups | 5714 | 19.7ms | 99% |
|
|
||||||
ldap_group_existence | | | |
|
ldap_group_existence | | | |
|
||||||
ldap_groups | 650 | 680.5ms | 99% |
|
ldap_groups | 650 | 680.5ms | 99% |
|
||||||
ldap_groups_byinclude | 1024 | | 83% |
|
ldap_groups_byinclude | 1024 | | 83% |
|
||||||
|
@@ -858,11 +858,6 @@ cache `"groups_bysubgroups"`::
|
|||||||
Caches the parent groups of a subgroup. If direct updates are made
|
Caches the parent groups of a subgroup. If direct updates are made
|
||||||
to the `account_group_includes` table, this cache should be flushed.
|
to the `account_group_includes` table, this cache should be flushed.
|
||||||
|
|
||||||
cache `"groups_subgroups"`::
|
|
||||||
+
|
|
||||||
Caches subgroups. If direct updates are made to the
|
|
||||||
`account_group_includes` table, this cache should be flushed.
|
|
||||||
|
|
||||||
cache `"ldap_groups"`::
|
cache `"ldap_groups"`::
|
||||||
+
|
+
|
||||||
Caches the LDAP groups that a user belongs to, if LDAP has been
|
Caches the LDAP groups that a user belongs to, if LDAP has been
|
||||||
|
@@ -363,16 +363,6 @@ The entries in the map are sorted by cache name.
|
|||||||
"entries": {},
|
"entries": {},
|
||||||
"hit_ratio": {}
|
"hit_ratio": {}
|
||||||
},
|
},
|
||||||
groups_subgroups": {
|
|
||||||
"type": "MEM",
|
|
||||||
"entries": {
|
|
||||||
"mem": 4
|
|
||||||
},
|
|
||||||
"average_get": "697.8us",
|
|
||||||
"hit_ratio": {
|
|
||||||
"mem": 82
|
|
||||||
}
|
|
||||||
},
|
|
||||||
"permission_sort": {
|
"permission_sort": {
|
||||||
"type": "MEM",
|
"type": "MEM",
|
||||||
"entries": {
|
"entries": {
|
||||||
@@ -477,7 +467,6 @@ The cache names are lexicographically sorted.
|
|||||||
"groups_bysubgroup",
|
"groups_bysubgroup",
|
||||||
"groups_byuuid",
|
"groups_byuuid",
|
||||||
"groups_external",
|
"groups_external",
|
||||||
"groups_subgroups",
|
|
||||||
"permission_sort",
|
"permission_sort",
|
||||||
"plugin_resources",
|
"plugin_resources",
|
||||||
"project_list",
|
"project_list",
|
||||||
|
@@ -29,14 +29,6 @@ public interface GroupIncludeCache {
|
|||||||
*/
|
*/
|
||||||
Collection<AccountGroup.UUID> getGroupsWithMember(Account.Id memberId);
|
Collection<AccountGroup.UUID> getGroupsWithMember(Account.Id memberId);
|
||||||
|
|
||||||
/**
|
|
||||||
* Returns the subgroups of a group.
|
|
||||||
*
|
|
||||||
* @param group the UUID of the group
|
|
||||||
* @return the UUIDs of all direct subgroups
|
|
||||||
*/
|
|
||||||
Collection<AccountGroup.UUID> subgroupsOf(AccountGroup.UUID group);
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the parent groups of a subgroup.
|
* Returns the parent groups of a subgroup.
|
||||||
*
|
*
|
||||||
@@ -50,7 +42,5 @@ public interface GroupIncludeCache {
|
|||||||
|
|
||||||
void evictGroupsWithMember(Account.Id memberId);
|
void evictGroupsWithMember(Account.Id memberId);
|
||||||
|
|
||||||
void evictSubgroupsOf(AccountGroup.UUID groupId);
|
|
||||||
|
|
||||||
void evictParentGroupsOf(AccountGroup.UUID groupId);
|
void evictParentGroupsOf(AccountGroup.UUID groupId);
|
||||||
}
|
}
|
||||||
|
@@ -22,7 +22,6 @@ import com.google.common.cache.LoadingCache;
|
|||||||
import com.google.common.collect.ImmutableList;
|
import com.google.common.collect.ImmutableList;
|
||||||
import com.google.common.collect.ImmutableSet;
|
import com.google.common.collect.ImmutableSet;
|
||||||
import com.google.common.collect.Streams;
|
import com.google.common.collect.Streams;
|
||||||
import com.google.gerrit.common.errors.NoSuchGroupException;
|
|
||||||
import com.google.gerrit.reviewdb.client.Account;
|
import com.google.gerrit.reviewdb.client.Account;
|
||||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||||
@@ -52,7 +51,6 @@ import org.slf4j.LoggerFactory;
|
|||||||
public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
||||||
private static final Logger log = LoggerFactory.getLogger(GroupIncludeCacheImpl.class);
|
private static final Logger log = LoggerFactory.getLogger(GroupIncludeCacheImpl.class);
|
||||||
private static final String PARENT_GROUPS_NAME = "groups_bysubgroup";
|
private static final String PARENT_GROUPS_NAME = "groups_bysubgroup";
|
||||||
private static final String SUBGROUPS_NAME = "groups_subgroups";
|
|
||||||
private static final String GROUPS_WITH_MEMBER_NAME = "groups_bymember";
|
private static final String GROUPS_WITH_MEMBER_NAME = "groups_bymember";
|
||||||
private static final String EXTERNAL_NAME = "groups_external";
|
private static final String EXTERNAL_NAME = "groups_external";
|
||||||
|
|
||||||
@@ -72,12 +70,6 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
|||||||
new TypeLiteral<ImmutableList<AccountGroup.UUID>>() {})
|
new TypeLiteral<ImmutableList<AccountGroup.UUID>>() {})
|
||||||
.loader(ParentGroupsLoader.class);
|
.loader(ParentGroupsLoader.class);
|
||||||
|
|
||||||
cache(
|
|
||||||
SUBGROUPS_NAME,
|
|
||||||
AccountGroup.UUID.class,
|
|
||||||
new TypeLiteral<ImmutableList<AccountGroup.UUID>>() {})
|
|
||||||
.loader(SubgroupsLoader.class);
|
|
||||||
|
|
||||||
cache(EXTERNAL_NAME, String.class, new TypeLiteral<ImmutableList<AccountGroup.UUID>>() {})
|
cache(EXTERNAL_NAME, String.class, new TypeLiteral<ImmutableList<AccountGroup.UUID>>() {})
|
||||||
.loader(AllExternalLoader.class);
|
.loader(AllExternalLoader.class);
|
||||||
|
|
||||||
@@ -88,7 +80,6 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private final LoadingCache<Account.Id, ImmutableSet<AccountGroup.UUID>> groupsWithMember;
|
private final LoadingCache<Account.Id, ImmutableSet<AccountGroup.UUID>> groupsWithMember;
|
||||||
private final LoadingCache<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> subgroups;
|
|
||||||
private final LoadingCache<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> parentGroups;
|
private final LoadingCache<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> parentGroups;
|
||||||
private final LoadingCache<String, ImmutableList<AccountGroup.UUID>> external;
|
private final LoadingCache<String, ImmutableList<AccountGroup.UUID>> external;
|
||||||
|
|
||||||
@@ -96,13 +87,10 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
|||||||
GroupIncludeCacheImpl(
|
GroupIncludeCacheImpl(
|
||||||
@Named(GROUPS_WITH_MEMBER_NAME)
|
@Named(GROUPS_WITH_MEMBER_NAME)
|
||||||
LoadingCache<Account.Id, ImmutableSet<AccountGroup.UUID>> groupsWithMember,
|
LoadingCache<Account.Id, ImmutableSet<AccountGroup.UUID>> groupsWithMember,
|
||||||
@Named(SUBGROUPS_NAME)
|
|
||||||
LoadingCache<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> subgroups,
|
|
||||||
@Named(PARENT_GROUPS_NAME)
|
@Named(PARENT_GROUPS_NAME)
|
||||||
LoadingCache<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> parentGroups,
|
LoadingCache<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> parentGroups,
|
||||||
@Named(EXTERNAL_NAME) LoadingCache<String, ImmutableList<AccountGroup.UUID>> external) {
|
@Named(EXTERNAL_NAME) LoadingCache<String, ImmutableList<AccountGroup.UUID>> external) {
|
||||||
this.groupsWithMember = groupsWithMember;
|
this.groupsWithMember = groupsWithMember;
|
||||||
this.subgroups = subgroups;
|
|
||||||
this.parentGroups = parentGroups;
|
this.parentGroups = parentGroups;
|
||||||
this.external = external;
|
this.external = external;
|
||||||
}
|
}
|
||||||
@@ -117,16 +105,6 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
|
||||||
public Collection<AccountGroup.UUID> subgroupsOf(AccountGroup.UUID groupId) {
|
|
||||||
try {
|
|
||||||
return subgroups.get(groupId);
|
|
||||||
} catch (ExecutionException e) {
|
|
||||||
log.warn("Cannot load members of group", e);
|
|
||||||
return Collections.emptySet();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Collection<AccountGroup.UUID> parentGroupsOf(AccountGroup.UUID groupId) {
|
public Collection<AccountGroup.UUID> parentGroupsOf(AccountGroup.UUID groupId) {
|
||||||
try {
|
try {
|
||||||
@@ -144,13 +122,6 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
|
||||||
public void evictSubgroupsOf(AccountGroup.UUID groupId) {
|
|
||||||
if (groupId != null) {
|
|
||||||
subgroups.invalidate(groupId);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void evictParentGroupsOf(AccountGroup.UUID groupId) {
|
public void evictParentGroupsOf(AccountGroup.UUID groupId) {
|
||||||
if (groupId != null) {
|
if (groupId != null) {
|
||||||
@@ -192,8 +163,7 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public ImmutableSet<AccountGroup.UUID> load(Account.Id memberId)
|
public ImmutableSet<AccountGroup.UUID> load(Account.Id memberId) throws OrmException {
|
||||||
throws OrmException, NoSuchGroupException {
|
|
||||||
GroupIndex groupIndex = groupIndexProvider.get();
|
GroupIndex groupIndex = groupIndexProvider.get();
|
||||||
if (groupIndex != null && groupIndex.getSchema().hasField(GroupField.MEMBER)) {
|
if (groupIndex != null && groupIndex.getSchema().hasField(GroupField.MEMBER)) {
|
||||||
return groupQueryProvider
|
return groupQueryProvider
|
||||||
@@ -213,26 +183,6 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
static class SubgroupsLoader
|
|
||||||
extends CacheLoader<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> {
|
|
||||||
private final SchemaFactory<ReviewDb> schema;
|
|
||||||
private final Groups groups;
|
|
||||||
|
|
||||||
@Inject
|
|
||||||
SubgroupsLoader(SchemaFactory<ReviewDb> sf, Groups groups) {
|
|
||||||
schema = sf;
|
|
||||||
this.groups = groups;
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
|
||||||
public ImmutableList<AccountGroup.UUID> load(AccountGroup.UUID key)
|
|
||||||
throws OrmException, NoSuchGroupException {
|
|
||||||
try (ReviewDb db = schema.open()) {
|
|
||||||
return groups.getSubgroups(db, key).collect(toImmutableList());
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
static class ParentGroupsLoader
|
static class ParentGroupsLoader
|
||||||
extends CacheLoader<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> {
|
extends CacheLoader<AccountGroup.UUID, ImmutableList<AccountGroup.UUID>> {
|
||||||
private final SchemaFactory<ReviewDb> schema;
|
private final SchemaFactory<ReviewDb> schema;
|
||||||
|
@@ -369,7 +369,6 @@ public class GroupsUpdate {
|
|||||||
for (AccountGroupById newIncludedGroup : newSubgroups) {
|
for (AccountGroupById newIncludedGroup : newSubgroups) {
|
||||||
groupIncludeCache.evictParentGroupsOf(newIncludedGroup.getIncludeUUID());
|
groupIncludeCache.evictParentGroupsOf(newIncludedGroup.getIncludeUUID());
|
||||||
}
|
}
|
||||||
groupIncludeCache.evictSubgroupsOf(parentGroupUuid);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -412,6 +411,5 @@ public class GroupsUpdate {
|
|||||||
for (AccountGroupById groupToRemove : subgroupsToRemove) {
|
for (AccountGroupById groupToRemove : subgroupsToRemove) {
|
||||||
groupIncludeCache.evictParentGroupsOf(groupToRemove.getIncludeUUID());
|
groupIncludeCache.evictParentGroupsOf(groupToRemove.getIncludeUUID());
|
||||||
}
|
}
|
||||||
groupIncludeCache.evictSubgroupsOf(parentGroupUuid);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Reference in New Issue
Block a user