----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68958/#review209360 -----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java Lines 180-183 (patched) <https://reviews.apache.org/r/68958/#comment293746> Why do we need these metrics? Isn't having the progress printed in the log enough? These are also printed in the log so we are incrementing the log size with duplicated messages. Btw, where are these metrics updated? Also, what if there is another snapshot, would these metrics contains only the snapshot committed or persisted? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java Line 422 (original), 422 (patched) <https://reviews.apache.org/r/68958/#comment293747> Use authorization objects which is how Sentry defines the HMS objects, isn't it? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 3437 (patched) <https://reviews.apache.org/r/68958/#comment293744> I think we should use time (like every 5min) instead of using a threshold. We don't know if persisting is in a lock situation or it is just too slow, so waiting until the # of objects has reached a specific number could be slower too. Btw, I don't think this time should be configurable. Once a snapshot is happning we cannot change the configuration, and once the snapshot is done we don't need to change it either. We have too many configurations already. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 3439 (patched) <https://reviews.apache.org/r/68958/#comment293745> What about printing the numbers of objects persisted as well? That number + the time would give us an idea of how much time is taken to persist an X number of objects. For instance: - 5:00am HMS snapshot persisting progress: authz_objs_persisted=100 authz_paths_persisted=500 authz_objs_size=1020 authz_paths_size=5500 - 5:05am HMS snapshot persisting progress: authz_objs=403 authz_paths=1020 authz_objs_size=1020 authz_paths_size=5500 You can deduct from the above that 303 authorization objects and 620 paths are persisted every 5min, so there is another 10m to finish. Btw, we should specify that this is an HMS snapshot. Just using 'number of objects' does not help users to understand what objects are being persisted. - Sergio Pena On Oct. 8, 2018, 10:16 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68958/ > ----------------------------------------------------------- > > (Updated Oct. 8, 2018, 10:16 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena. > > > Bugs: SENTRY-2419 > https://issues.apache.org/jira/browse/SENTRY-2419 > > > Repository: sentry > > > Description > ------- > > Process of persisting the snapshot might take longer based on the snapshot > size. It would be helpfull to to be able to understand where the sentry > sentry stands in the process of persisting. > > > > Adding a Metric for this will be helpfull in debugging. > > > Diffs > ----- > > > sentry-core/sentry-core-common/src/main/java/org/apache/sentry/service/common/ServiceConstants.java > e90fe2df4 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java > d34340cd6 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java > b6dca7aa8 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 1722109e3 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreInterface.java > 3a68eb6bf > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java > 059621a68 > > > Diff: https://reviews.apache.org/r/68958/diff/2/ > > > Testing > ------- > > > Thanks, > > Arjun Mishra > >
