> On June 16, 2016, 7:41 p.m., Colin McCabe wrote: > > 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/diff/1/?file=1420638#file1420638line29> > > > > 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?
Yeah, makes sense. Will update the name. > On June 16, 2016, 7:41 p.m., Colin McCabe wrote: > > 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/diff/1/?file=1420639#file1420639line249> > > > > Should be CREATE_TIME_MS to make it clear it's in milliseconds? Will fix. > On June 16, 2016, 7:41 p.m., Colin McCabe wrote: > > 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/diff/1/?file=1420638#file1420638line71> > > > > 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. Does not seem a must to define equal and hashCode for now, but probably would be needed later. Sorry I did not get what is the much simpler way to do? > On June 16, 2016, 7:41 p.m., Colin McCabe wrote: > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java, > > line 649 > > <https://reviews.apache.org/r/48770/diff/1/?file=1420636#file1420636line649> > > > > 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. Agree. I will remove the function now. - Hao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48770/#review138066 ----------------------------------------------------------- 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 > >
