----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48770/#review138066 -----------------------------------------------------------
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java (line 419) <https://reviews.apache.org/r/48770/#comment203272> Should this be authzObjToEntries? sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java (line 645) <https://reviews.apache.org/r/48770/#comment203273> It seems like the Map returned by this function could be extremely large if we have many mappings. Where is this intended to be used? It seems like it would be better to add accessors to the object itself, rather than copying the complete data set. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathsUpdate.java (line 26) <https://reviews.apache.org/r/48770/#comment203274> typo: "can re-enhance" sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathsUpdate.java (line 29) <https://reviews.apache.org/r/48770/#comment203275> I'm not sure if "MSentryPathsUpdate" is the best name for this. It's not really an "update", it's the mapping between a hive object and its paths, right? Perhaps MHivePathMapping? or similar? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathsUpdate.java (line 71) <https://reviews.apache.org/r/48770/#comment203276> Is it necessary to define hashCode for this object? If so, we could do it in a much simpler way than this. Currently, if authzObjName is NULL, then this will always return 31. Otherwise, the only randomness comes from authzObjName#hashCode. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo (line 249) <https://reviews.apache.org/r/48770/#comment203277> Should be CREATE_TIME_MS to make it clear it's in milliseconds? - Colin McCabe On June 16, 2016, 2:27 a.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48770/ > ----------------------------------------------------------- > > (Updated June 16, 2016, 2:27 a.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/MSentryPathsUpdate.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 > >
