----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71007/#review216348 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java Line 1699 (original), 1699 (patched) <https://reviews.apache.org/r/71007/#comment303528> Please consider renaming policies->policyIds security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java Line 1700 (original), 1700 (patched) <https://reviews.apache.org/r/71007/#comment303529> Please consider putting lines from 1700-1706 (inclusive) under a check - if (CollectionUtils.isNotEmpty(policies)) - security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java Lines 2065 (patched) <https://reviews.apache.org/r/71007/#comment303532> Is it likely that policy or service could be null? If so, consider putting all logic here under a check : if (service != null && policy != null). security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java Lines 2069 (patched) <https://reviews.apache.org/r/71007/#comment303530> Consider moving this 2069-2071 to beginning of the function (without referencing policy object). security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java Lines 2082 (patched) <https://reviews.apache.org/r/71007/#comment303533> Consider printing policy id and name as debug message. security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java Lines 2097 (patched) <https://reviews.apache.org/r/71007/#comment303531> Please replace this with debug statement to print exit from function security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java Line 2437 (original), 2490 (patched) <https://reviews.apache.org/r/71007/#comment303534> Why is this change needed? security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java Lines 3010 (patched) <https://reviews.apache.org/r/71007/#comment303535> Consider moving lines 3010-3013 to after line 3008. security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java Line 111 (original) <https://reviews.apache.org/r/71007/#comment303536> Why is this change needed? security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java Line 262 (original) <https://reviews.apache.org/r/71007/#comment303537> Why is this change needed? security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java Line 177 (original), 190 (patched) <https://reviews.apache.org/r/71007/#comment303538> Please consider renaming this to findPolicyIdsByServiceNameAndZoneId() security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java Line 2694 (original), 2692 (patched) <https://reviews.apache.org/r/71007/#comment303540> deletePolicy(policyId) is used to delete policy from here. Does this need to be replaced with deletePolicy(policy,service) call too? Please review. security-admin/src/main/resources/META-INF/jpa_named_queries.xml Line 300 (original), 304 (patched) <https://reviews.apache.org/r/71007/#comment303539> Consider renaming this to XXPolicy.findPolicyIdsByServiceNameAndZoneId - Abhay Kulkarni On July 3, 2019, 3:39 p.m., Pradeep Agrawal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71007/ > ----------------------------------------------------------- > > (Updated July 3, 2019, 3:39 p.m.) > > > Review request for ranger, Ankita Sinha, bhavik patel, Gautam Borad, Abhay > Kulkarni, Madhan Neethiraj, Mehul Parikh, Nikhil P, Nitin Galave, Ramesh > Mani, Sailaja Polavarapu, and Velmurugan Periasamy. > > > Bugs: RANGER-2493 > https://issues.apache.org/jira/browse/RANGER-2493 > > > Repository: ranger > > > Description > ------- > > ** Problem Statement ** Ranger takes an extremely long time to override a > service with many policies, will crash ranger if admin JVM heap size is the 1 > GB default > > > Diffs > ----- > > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > bf50df3a8 > security-admin/src/main/java/org/apache/ranger/common/db/BaseDao.java > 51c0de56c > security-admin/src/main/java/org/apache/ranger/db/XXPolicyDao.java > baf6b6ea4 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > 171d73bfa > security-admin/src/main/resources/META-INF/jpa_named_queries.xml eb0384b14 > security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java > dc845cf80 > security-admin/src/test/java/org/apache/ranger/rest/TestServiceREST.java > 9b9aa8377 > > > Diff: https://reviews.apache.org/r/71007/diff/1/ > > > Testing > ------- > > Tested 500 policy import json and unable to reproduce JVM issue. > > > Thanks, > > Pradeep Agrawal > >
