> 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. > > Madhan Neethiraj wrote: > 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
Okay - the original review you suggested to keep it in the "optimized" scripts (for new installations) while not having the patch on upgrade to preserve HA deployments. I think in this case it's ideal to reserve a patch number for this since if we don't reserve a patch number, there won't be any indication in the x_db_version_h that these columns will be removed, and then when upgrading to 3.x it will re-run this patch. It should still work fine but it's less efficient/ideal. Let me know what you prefer (or if you think we should remove the DB changes entirely from 2.4, including the "optimized" scripts) - Andrew ----------------------------------------------------------- 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 > >
