----------------------------------------------------------- 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 > >