----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48770/#review138864 -----------------------------------------------------------
Thanks Haohao. Left some minor comments. sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java (line 122) <https://reviews.apache.org/r/48770/#comment204065> Not sure why here it's final. If means it is not mutable, maybe use a different datastructure, such as, ImmutableMap (Google). sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java (line 342) <https://reviews.apache.org/r/48770/#comment204063> Whitespace. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java (line 25) <https://reviews.apache.org/r/48770/#comment204068> Transactional database backed table for .... sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java (line 95) <https://reviews.apache.org/r/48770/#comment204069> Just want to confirm if want to double check path content for equality? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo (line 248) <https://reviews.apache.org/r/48770/#comment204070> name="value" maybe keep it consistent by using whitespace or no-whitespace. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 64) <https://reviews.apache.org/r/48770/#comment204071> import alhpabetic asending order? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 2002) <https://reviews.apache.org/r/48770/#comment204073> Just want to confirm there is no two authzToPaths objects with the same getAuthzObjName case; if so, might check existence then update value. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 2008) <https://reviews.apache.org/r/48770/#comment204074> Didn't find where to set rollbackTransaction = false? sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java (line 2084) <https://reviews.apache.org/r/48770/#comment204076> Just curious, in this case, will the below two cases are the same? hdfs://nameservice/user/hive/warehouse/db1.db/table1 and db1.db/table1? whether the same or not, could add a test case to check? - Anne Yu On June 17, 2016, 8:32 p.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48770/ > ----------------------------------------------------------- > > (Updated June 17, 2016, 8:32 p.m.) > > > Review request for sentry, Colin McCabe and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > SENTRY-1325: Store HMSPaths in Sentry DB to allow fast failover > > Change-Id: Icfb86d86b8cdbbf786c48780cc3ee8ec8a2b2698 > > Added schema for storing HMSPaths: MSentryPathsUpdate. It has hiveObj, paths, > createTime. > Added datastore APIs: retrieveFullPathsImage, and createSentryPathsUpdate. > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java > 6d2ab23c3f65af5430ea02563e13c4c4c49aa1c6 > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java > e759ff1229353570c5fc1a8547f47127e2dabff1 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > b3b949400c2a6dd5797a678da8966617b0598cca > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > b7ef0e94fac911e97e4ae9ba1cde49edbe1eb62b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > bc7fe12b732e523ad6f50fd1daf5078cc37614f1 > > Diff: https://reviews.apache.org/r/48770/diff/ > > > Testing > ------- > > Added a test case in TestSentryStore#testPathsUpdate. > > > Thanks, > > Hao Hao > >
