[ https://issues.apache.org/jira/browse/RANGER-3014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17208740#comment-17208740 ]
Mahesh Hanumant Bandal commented on RANGER-3014: ------------------------------------------------ Hi [~RickyMa], As I have mentioned earlier "*User lookup in permission tab breaks current functionality*". I have tested with RANGER-2789 patch and also with your [latest patch|https://reviews.apache.org/r/72926/] . Please refer to following image !ranger-crashed while user-lookup in permission tab.png|width=448,height=198! . When ranger tries to lookup for users in permission tab, and if we keep typing/deleting few letters in "Select and Add User" textbox; it sends requests for user search on keyPressEvent from UI side. After sometime all request starts failing and ranger stops responding. Also, I checked cpu utilization and it reaches over 370%. Please refer to this image !cpu_utilization_for_user_lookup.png|width=210,height=176! Apart from this, on UI while loading User Tab with default pageSize=25. It takes 2.92 seconds. Please refer image below !with RANGER-2789 and RANGER-3014 .png|width=330,height=147! *Following are analysis after reverting RANGER-2789* : If we revert RANGER-2789 it took *319.15 milliseconds* with default pageSize=25, which is comparatively very less. Please refer following image. !After Reverting RANGER-2789.png|width=321,height=143! Also, user lookup in permission tab also works smoothly without failing. Please refer following image !Reverted RANGER-2789- userlookup in permission tab.png|width=321,height=144! However, *pageSize=1000 took 16 seconds* to respond but this is related to RANGER-2789 and this performance issue can be tracked separately. For now, we need revert this patch as it breaks current functionality as I explained. Thanks, - Mahesh Bandal > fix for RANGER-2789 breaks current functionality > ------------------------------------------------ > > Key: RANGER-3014 > URL: https://issues.apache.org/jira/browse/RANGER-3014 > Project: Ranger > Issue Type: Bug > Components: admin > Reporter: Georgi Ivanov > Assignee: Mahesh Hanumant Bandal > Priority: Major > Attachments: > 0001-RANGER-3014-fix-for-RANGER-2789-breaks-current-funct.patch, After > Reverting RANGER-2789.png, Reverted RANGER-2789- userlookup in permission > tab.png, cpu_utilization_for_user_lookup.png, ranger-crashed while > user-lookup in permission tab.png, with RANGER-2789 and RANGER-3014 .png > > Time Spent: 10m > Remaining Estimate: 0h > > Since we upgraded to Ranger 2.1.0 in our dev env, we've noticed that user > list page in Ranger Admin UI is not showing (or it is very very slow - in the > order of tens of minutes). > Looking at the commit history we found that the reason was commit > *f45054d1b9* which was meant as a performance improvement for RANGER-2789. > Our ranger usersync fetches users and groups from AD. Our tree is huge, here > are some stats: > {code:java} > select count(*) from x_user; > 43368 > select count(*) from x_portal_user; > 43366 > select count(*) from x_group; > 17865 > select count(*) from x_group_users; > 366180 {code} > > Looking at the commit *f45054d1b9* what it meant to solve is perform a user > lookup and fetching user info such as attributes and group membership in > bulk, instead of doing it in a loop, one by one. In order to do that it > provided couple of methods and also an override for searchXUsers in > service/XUserService.java (before we used the parent method in > service/XUserServiceBase.java). > > The new searchXUsers method (which gets invoked when we call > /service/xusers/users REST API, calls populateViewBeans (another new method). > It calls the parent method populateViewBeans in XUserServiceBase.java which > build a hashmap or users and calls an override of populateViewBeans with > input hashmap > {code:java} > + public List<VXUser> populateViewBeans(List<XXUser> resources) { > + List<VXUser> viewBeans = new ArrayList<>(); > + if (CollectionUtils.isNotEmpty(resources)) { > + Map<XXUser, VXUser> resourceViewBeanMap = new > HashMap<>(resources.size()); > + Map<VXUser, XXUser> viewBeanResourceMap = new > HashMap<>(resources.size()); > + for (XXUser resource : resources) { > + VXUser viewBean = createViewObject(); > + > viewBean.setCredStoreId(resource.getCredStoreId()); > + > viewBean.setDescription(resource.getDescription()); > + viewBean.setName(resource.getName()); > + viewBean.setStatus(resource.getStatus()); > + resourceViewBeanMap.put(resource, viewBean); > + viewBeanResourceMap.put(viewBean, resource); > + viewBeans.add(viewBean); > + } > + populateViewBeans(resourceViewBeanMap); > + mapEntityToViewBeans(viewBeanResourceMap); > + } > + return viewBeans; > + } > + > + protected void populateViewBeans(Map<XXUser, VXUser> > resourceViewBeanMap) { > + mapBaseAttributesToViewBeans(resourceViewBeanMap); > + } {code} > > This in turns calls mapBaseAttributesToViewBeans, which calls > daoManager.getXXPortalUser().findAllXPortalUser() and it pulls all users (no > matter that we limit the users with a REST call to 25 by default) > That's one thing that hampers performance. However the biggest issue is this: > {code:java} > + @Override > + public List<VXUser> populateViewBeans(List<XXUser> xUsers) { > + List<VXUser> vObjList = super.populateViewBeans(xUsers); > + if (CollectionUtils.isNotEmpty(vObjList) && > CollectionUtils.isNotEmpty(xUsers) && xUsers.size() == vObjList.size()) { > + Map<Long, VXUser> xUserIdVObjMap = new > HashMap<>(xUsers.size()); > + for (int i = 0; i < xUsers.size(); ++i) { > + VXUser vObj = vObjList.get(i); > + XXUser xUser = xUsers.get(i); > + vObj.setIsVisible(xUser.getIsVisible()); > + xUserIdVObjMap.put(xUser.getId(), vObj); > + } > + populateGroupList(xUserIdVObjMap); > + } > + return vObjList; > + } {code} > We call populateGroupList on the list of users (by default 25) but we call a > new method that accepts a map as an input. Inside that method we call > {code:java} > List<XXGroupUser> allXXGroupUsers = daoManager.getXXGroupUser().getAll(); > {code} > Which in our case will pull all 366180 group to user membership mappings from > x_group_users table. > Next we filter through the whole group list just to find all users who have > memberships in those group (but we traverse the whole group membership list) > {code:java} > if (MapUtils.isNotEmpty(xUserIdVObjMap) && > CollectionUtils.isNotEmpty(allXXGroupUsers)) { > Map<Long, List<XXGroupUser>> userIdXXGroupUserMap = new > HashMap<>(xUserIdVObjMap.size()); > for (Map.Entry<Long, VXUser> xUserIdVXUserEntry : > xUserIdVObjMap.entrySet()) { > Long xUserId = xUserIdVXUserEntry.getKey(); > List<XXGroupUser> xxGroupUsers = allXXGroupUsers > .stream() > .filter(xXGroupUser -> > Objects.equals(xXGroupUser.getUserId(), xUserId)) > .collect(Collectors.toList()); > userIdXXGroupUserMap.put(xUserId, xxGroupUsers); {code} > This is what happens when we open Ranger and go the User UI. We make a > paginated request to view the first 25 users from the DB, but actually what > ranger does is pulling all users from the DB and also traversing the whole > group-to-user membership list. > > When reverting the commit to go to the old behaviour things went back to > normal. We understand the rationale against this but the implementation > introduces more bugs than it tries to solve. -- This message was sent by Atlassian Jira (v8.3.4#803005)