> On Feb. 27, 2023, 6:21 a.m., Madhan Neethiraj wrote: > > Andrew - the changes look good. As discussed earlier in this review, DB > > changes to remove unused columns should be done only in next major release > > - 3.0 i.e., in master branch. I suggest to split this patch into two parts: > > 1) Java changes to remove references to unused fields. This patch should > > be committed in both ranger-2.4 and master branches > > 2) DB changes to remove unused columns. This patch should be committed > > only in master branch > > Andrew Luo wrote: > Thanks - (1) is in this patch, and (2) is in thie patch but I've modified > db_setup.py to skip the patch (by skipping patches with SKIP at the end of > the ID) and also commenting out the contents of the patch. I wanted to do > this to ensure the patch is there for reference (the only difference is that > on master we will rename 061SKIP-... to 061-... and uncomment the contents of > the patch). If you prefer to remove it entirely - that's fine also, but we > will have to make sure nobody uses the 061 patch number on the 2.4 branch.
Thanks for the details of db_setup.py skipping DB updates in 2.4 branch; I missed this. Considering 2.4 will only have updates in Java classes, is there a need reserve a patch number (like 061)? DB patch can be merged in the master branch with the most recent patch number. At the time of release from master branch, the patch number might need to be revisited if any maintanence release (2.5/..) ended up using that patch number. I suggest splitting this patch into 2 parts: - Java class changes: to be merged in master and ranger-2.4 branches - DB schema changes: to be merged in master branch only - Madhan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74301/#review225220 ----------------------------------------------------------- On Feb. 24, 2023, 1:23 p.m., Andrew Luo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74301/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2023, 1:23 p.m.) > > > Review request for ranger and Pradeep Agrawal. > > > Bugs: RANGER-2713 > https://issues.apache.org/jira/browse/RANGER-2713 > > > Repository: ranger > > > Description > ------- > > XXPolicyRef objects have fields such as create time, update time, added by > user ID, updated by user ID, but there fields are entirely useless since they > are all copied from the XXPolicy object. In addition, while improving > performance for creation of policies with large numbers of users, we > discovered that a lot of time was being spent in JPA converting these Date > objects especially. After removing these fields we saw a significant > performance improvement (a secondary benefit is less database space usage). > > We previously tried this commit and it caused a few issues (some typos on SQL > Server and SQL Anywhere, as well as upgrade scenarios - however this has been > fixed). > > > Diffs > ----- > > security-admin/db/mysql/optimized/current/ranger_core_db_mysql.sql > 9a79fe8ad > > security-admin/db/mysql/patches/061SKIP-drop-audit-columns-from-policy-ref-tables.sql > PRE-CREATION > security-admin/db/oracle/optimized/current/ranger_core_db_oracle.sql > fd6cec9a7 > > security-admin/db/oracle/patches/061SKIP-drop-audit-columns-from-policy-ref-tables.sql > PRE-CREATION > security-admin/db/postgres/optimized/current/ranger_core_db_postgres.sql > 4d5a8cedf > > security-admin/db/postgres/patches/061SKIP-drop-audit-columns-from-policy-ref-tables.sql > PRE-CREATION > > security-admin/db/sqlanywhere/optimized/current/ranger_core_db_sqlanywhere.sql > 3ed2a5b9c > > security-admin/db/sqlanywhere/patches/061SKIP-drop-audit-columns-from-policy-ref-tables.sql > PRE-CREATION > security-admin/db/sqlserver/optimized/current/ranger_core_db_sqlserver.sql > ca8f7da1f > > security-admin/db/sqlserver/patches/061SKIP-drop-audit-columns-from-policy-ref-tables.sql > PRE-CREATION > security-admin/scripts/db_setup.py 24502f4fb > security-admin/src/main/java/org/apache/ranger/biz/PolicyRefUpdater.java > 6cc3509d8 > security-admin/src/main/java/org/apache/ranger/biz/RoleRefUpdater.java > 421b2312d > > security-admin/src/main/java/org/apache/ranger/biz/SecurityZoneRefUpdater.java > 4cfe62701 > > security-admin/src/main/java/org/apache/ranger/entity/XXPolicyRefAccessType.java > 6af8f99f4 > > security-admin/src/main/java/org/apache/ranger/entity/XXPolicyRefCondition.java > 4f4409d6a > > security-admin/src/main/java/org/apache/ranger/entity/XXPolicyRefDataMaskType.java > cb926740e > security-admin/src/main/java/org/apache/ranger/entity/XXPolicyRefGroup.java > 32a1b9f24 > > security-admin/src/main/java/org/apache/ranger/entity/XXPolicyRefResource.java > 115064621 > security-admin/src/main/java/org/apache/ranger/entity/XXPolicyRefRole.java > 7aee502e0 > security-admin/src/main/java/org/apache/ranger/entity/XXPolicyRefUser.java > 8dfb92833 > security-admin/src/main/java/org/apache/ranger/entity/XXRoleRefGroup.java > 22b944791 > security-admin/src/main/java/org/apache/ranger/entity/XXRoleRefRole.java > 30867e28d > security-admin/src/main/java/org/apache/ranger/entity/XXRoleRefUser.java > a5b17f716 > > security-admin/src/main/java/org/apache/ranger/entity/XXSecurityZoneRefGroup.java > 0ae6b2ffc > > security-admin/src/main/java/org/apache/ranger/entity/XXSecurityZoneRefResource.java > 3d7197a16 > > security-admin/src/main/java/org/apache/ranger/entity/XXSecurityZoneRefService.java > a2cacc674 > > security-admin/src/main/java/org/apache/ranger/entity/XXSecurityZoneRefTagService.java > c67a4648d > > security-admin/src/main/java/org/apache/ranger/entity/XXSecurityZoneRefUser.java > 4af242fd3 > > security-admin/src/main/java/org/apache/ranger/service/XPortalUserService.java > 85e457efa > security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java > 691ab52b3 > > > Diff: https://reviews.apache.org/r/74301/diff/4/ > > > Testing > ------- > > Fresh install and upgrade tested on all 5 databases (tested service and > policy creation) > > Also tested user deletion on MySQL: > > 1. Created new user > 2. Logged in as new user, created a policy > 3. Logged back in as original user, delete user that created policy > 4. Verified no errors, policy is still present/correct > > Also searched the source code and JPA queries xml for other references to > these fields > > > Thanks, > > Andrew Luo > >
