Alon Bar-Lev has posted comments on this change. Change subject: aaa: Fix sync ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/28561/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DbUserCacheManager.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DbUserCacheManager.java: Line 169: } Line 170: } Line 171: } Line 172: Line 173: Map<String, DirectoryGroup> cache = new HashMap<>(); I still think you should go via users first, as it will reduce the # of fetches of groups. Line 174: populateRecrusiveGroups(authz, cache, toFetch); Line 175: Line 176: Line 177: Line 170: } Line 171: } Line 172: Line 173: Map<String, DirectoryGroup> cache = new HashMap<>(); Line 174: populateRecrusiveGroups(authz, cache, toFetch); there is some loop missing.... as here you call populateRecrusiveGroups which will resolve up to 2nd level... we need to loop until we have nothing to resolve. where is it done? Line 175: Line 176: Line 177: Line 178: for (DirectoryUser directoryUser : directoryUsers) { Line 220: } else { Line 221: for (DirectoryGroup group : toCache) { Line 222: if (!cache.containsKey(group.getId())) { Line 223: Set<DirectoryGroup> flatGroups = new HashSet<>(); Line 224: DirectoryGroup fetchedGroup = AuthzUtils.findGroupById(authz, group.getNamespace(), group.getId(), true); hmmm.... starting to understand, I think. but please every time you fetch groups, you need to fetch them in block, to reduce the number of queries. never fetch a single group, always prepare a list of ids. Line 225: cache.put(fetchedGroup.getId(), fetchedGroup); Line 226: DirectoryUtils.flatGroups(flatGroups, fetchedGroup.getGroups()); Line 227: for (DirectoryGroup fetchedMemberOf : flatGroups) { Line 228: cache.put(fetchedMemberOf.getId(), fetchedMemberOf); http://gerrit.ovirt.org/#/c/28561/6/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalAuthz.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalAuthz.java: Line 123: Authz.PrincipalRecord.ID, Line 124: configuration.getProperty("config.authz.user.id") Line 125: ).mput( Line 126: Authz.PrincipalRecord.GROUPS, Line 127: Collections.emptyList() again, no need. Line 128: ); Line 129: Line 130: } Line 131: -- To view, visit http://gerrit.ovirt.org/28561 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id49b51517a967c7a83e8e73f52181673baa31700 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
