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




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 107 (patched)
<https://reviews.apache.org/r/69076/#comment298052>

    Can you make configurarble? It might be helpfull.
     What do you say?



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 279 (patched)
<https://reviews.apache.org/r/69076/#comment298050>

    I dont't a value is printing partition-id. Instead we should be looging 
partion name. Yes, it will be big. Information that we are trying to log will 
be any way high regardless.



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 299-300 (patched)
<https://reviews.apache.org/r/69076/#comment298053>

    Change "Fetch partition objects for" to "Fetching partitions for ". This 
comment applies to tabletask and dbtask as well.



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 319 (patched)
<https://reviews.apache.org/r/69076/#comment298054>

    Printing partitionId doesn't tell you anything. Looking at the ID you can 
not derive the parition that has null storage descriptot. You should print 
partition name.



sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
Lines 555 (patched)
<https://reviews.apache.org/r/69076/#comment298051>

    This doesn't look correct because results might be still getting appended 
by the tasks.(PartitionTask/TableTask/DbTask)
    
    You should instead print the number of objects for which paths are already 
fetched. Number of objects here include database and tables.
    
    You can use fullSnapshot.size(). This will give the total number of 
databases and tables fetches fetched so far.
    
    This is good enough.


- kalyan kumar kalvagadda


On Jan. 24, 2019, 6:24 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69076/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2019, 6:24 p.m.)
> 
> 
> Review request for sentry, Haley Reeve, kalyan kumar kalvagadda, Na Li, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> When sentry is fetching snapshot from HMS, it should log periodically on 
> where it stands in the snapshot process. This will help person debugging it 
> and help him understand the progress.
> 
>  
> 
> This is important as this process could take magnitude of minutes when the 
> HMS data is huge.
> 
> 
> Diffs
> -----
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java
>  534bb51c1 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/FullUpdateInitializer.java
>  4ff3dc91a 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  4baeb6725 
> 
> 
> Diff: https://reviews.apache.org/r/69076/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Reply via email to