> On Dec. 11, 2018, 8:56 p.m., Sergio Pena wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 3451-3455 (patched) > > <https://reviews.apache.org/r/69530/diff/5/?file=2113089#file2113089line3451> > > > > Why to use two different methods instead of using only the compact? > > Doesn't do the same thing?
Sergio we found a case where we couldn't delete paths without knowing the objects. Case with 1 path multiple objects. Thats why two methods. If we detect a single path is associated with multiple objects we do the old approach of getting the objects first and then the paths. Else we delete the paths directly > On Dec. 11, 2018, 8:56 p.m., Sergio Pena wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 3485 (patched) > > <https://reviews.apache.org/r/69530/diff/5/?file=2113089#file2113089line3485> > > > > I did not understand this. Why do you to read all the > > MAuthzPathsMapping objects? That might be a huge number considering the > > thousand of objects (or millions) that HMS can have and that several > > snapshots may be stored in the DB due to old bugs. This would read all of > > them. > > > > Isn't it going to perform bad? We are NOT reading all the MAuthzPathsMapping objects. We are reading all MAuthzPathsMapping objects for a given OBJECT NAME. So if there are multiple snapshots we get all entries of MAuthzPathsMapping for a given name > On Dec. 11, 2018, 8:56 p.m., Sergio Pena wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 3590 (patched) > > <https://reviews.apache.org/r/69530/diff/5/?file=2113089#file2113089line3590> > > > > This method reads all MAuthzPathsMapping object to check only if there > > are multiple MAuthzPathsMapping relations to a set of MAuthzPath objects. > > > > Is there a way to check that with less data read from the db? Take into > > account that there might be thousdans of MAuthzPathsMapping. Its a smiple query. I don't think this will impact performance > On Dec. 11, 2018, 8:56 p.m., Sergio Pena wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 3592-3595 (patched) > > <https://reviews.apache.org/r/69530/diff/5/?file=2113089#file2113089line3592> > > > > This code is repeated in the Exhaustive methods. Isn't it better to > > pass the snapshotID as parameter instead? You can get the snapshotID once > > from the ...Core() method. Ok works. - Arjun ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69530/#review211208 ----------------------------------------------------------- On Dec. 11, 2018, 8:16 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69530/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2018, 8:16 p.m.) > > > Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and > Sergio Pena. > > > Bugs: SENTRY-2476 > https://issues.apache.org/jira/browse/SENTRY-2476 > > > Repository: sentry > > > Description > ------- > > Right now when we process a drop partition event, we fetch each path object > for paths_mapping object then find the one we want to delete and then delete > it. We should avoid fetching all objects and directly delete the path that > needs to be deleted > > This affects, deleteAuthzPathsMapping, renameAuthzPathsMapping, > updateAuthzPathsMapping, and addAuthzPathsMappingCore methods that are known > at the moment > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/SentryConstants.java > d8c1061d3 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java > f5802d701 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > e2d6c85ac > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > ca8c41610 > > > Diff: https://reviews.apache.org/r/69530/diff/5/ > > > Testing > ------- > > $ mvn -f sentry-service/sentry-service-server/pom.xml test > -Dtest=TestNotificationProcessor > > > Thanks, > > Arjun Mishra > >
