----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68727/#review209067 -----------------------------------------------------------
Changes look good. But I'd like to know two things before approving it: 1. Could you verify Lina's concerns (see JIRA) about what would happen if a partition or table location changes in the NotificationProcessor? If a partition location that has the same prefix as the table changes to be out of the table location, would this affect the HDFS ACLs? 2. Could you run a benchmark with and without this patch to see the improvement in numbers? - Sergio Pena On Sept. 24, 2018, 10:28 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68727/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2018, 10:28 p.m.) > > > Review request for sentry, Alexander Kolbasov, Arjun Mishra, Na Li, and > Sergio Pena. > > > Bugs: SENTRY-2306 > https://issues.apache.org/jira/browse/SENTRY-2306 > > > Repository: sentry > > > Description > ------- > > Ignore the partitions entries which are located in default location as they > share the same permissions as the table. This will reduce the snapshot size > decreasing the time to persist the snapshot and also time to send the full > update to Namenode. This is important as the partition information has lion’s > share in a HMS Full snapshot. > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 1eda41b2b6bd940a404cc1ba09a861fe783ead04 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java > 0b4f4aa24bd3002c50bf4d80a6fa361f66052973 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java > 3e27d1bbee9bea9a60e7f7e012a367957610826c > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryService.java > b3a4934dfc523d14eec216df00a6b7597c66c166 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateInitializer.java > 589acbed12855ff09309a04c9214f8daf87ea1de > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > 3d7fbe3fea333d58e46cd721d610c7d8050c9de4 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java > 574bc4b0432824abc81fe3c0dd30d1fe01f012e5 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentryService.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java > 9fa42f2de41b6700a18a358f8d5e442104dd0585 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/minisentry/SentrySrv.java > 0e82b86a727f578013a63c005aa05279790344f1 > > > Diff: https://reviews.apache.org/r/68727/diff/4/ > > > Testing > ------- > > Made sure all the existing tests passed and also added unit and e2e tests to > verify the new behavior. > > > Thanks, > > kalyan kumar kalvagadda > >
