> On April 18, 2017, 4:14 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Lines 257 (patched)
> > <https://reviews.apache.org/r/58412/diff/2/?file=1693609#file1693609line257>
> >
> >     how long will the notification ID overflow? This ID is saved in DB, and 
> > if it wraps within a reasonable time for big customers, we need to handle 
> > the overflow case. 
> >     
> >     For example, instead of check "if (eventId.getEventId() > 
> > currentEventID)" we can have a function "CompareEventId(eventIdA, 
> > eventIdB)". if eventId A is older than eventId B, returns -1, if equal, 
> > returns 0, and newer, returns 1. 
> >     
> >     You can have simple implementation, but later can change to handle 
> > overflow.

That's a good point to ask how overflow is handled. However, I think overflow 
should be handled by Hive metastore before they even generate a new 
notification log. And notify its client such as Sentry. It would be interesting 
to check if HMS is handling for overflow.


> On April 18, 2017, 4:14 p.m., Na Li wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
> > Line 343 (original), 348 (patched)
> > <https://reviews.apache.org/r/58412/diff/2/?file=1693609#file1693609line348>
> >
> >     If this function only throw SentryInvalidHMSEventException, why not 
> > mark the function with this specific exception instead of "Exception"?

notificationProcessor.processXXX would throw Exception coming from SentryStore.


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58412/#review172212
-----------------------------------------------------------


On April 18, 2017, 7:34 a.m., Hao Hao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58412/
> -----------------------------------------------------------
> 
> (Updated April 18, 2017, 7:34 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1587: Refactor SentryStore transaction to persist a single path 
> transcation bundled with corresponding delta path change
> 
> Change-Id: Ice77f631e92c0e74b42ae644e1b2f90c6089e62f
> 
> 
> Diffs
> -----
> 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java
>  90aaaef0e15306627d7108f12a74a29848055c0b 
>   
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
>  14e967aa1065f16e8d4c3f61db2f9055959fa9e6 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java
>  c22364fa26c80415827313f4d26f6c53b71b6f6c 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  08520f30f3529af88d7dab9ae3623f28f0e43558 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  16676fb13b0d5015aefe892a6f7e46812ea75124 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
>  87e329588990be129197d598dd1db4487f8e0f25 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationAdvanced.java
>  24ab1a8b392f23bc75759733bef7cecd4bc2ac34 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  7769f24bb4c062016084bcafe7d50a0f0e013824 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  0e97466c9ba6973d8e787819e292c3b826472b38 
> 
> 
> Diff: https://reviews.apache.org/r/58412/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hao Hao
> 
>

Reply via email to