You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Hao Hao <ha...@cloudera.com> on 2017/03/01 20:20:31 UTC
Re: Review Request 55904: SENTRY-1612: HMSFollower should persist
full HMS snapshot into SentryDB if there is not one.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55904/
-----------------------------------------------------------
(Updated March 1, 2017, 8:20 p.m.)
Review request for sentry.
Repository: sentry
Description
-------
After reads a point-in-time full HMSPaths snapshot from HMS and record the corresponding
notification ID, HMSFollwer should persist it into Sentry DB.
Otherwise if a full snapshot is stored in DB, read the corresponding currentEventID from sentry store.
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java f95dd9405bce6f5fb6ec0e0c708a57a3093a1db0
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java ffb07560d7c40fb14cef77950012c2c81fa3fd28
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestFullUpdateInitializer.java 0bb6f665ae51f7dd63a9bee545150a85f42274a5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java 0ca7fe254008b021e86edb42c70fe002eb801930
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b9272bc80ea473685b90f6c30dc8e0b0dff7b9a9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java bdbb0cc3ff639bf2e5c3725e6ebf1cc641c01374
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 0e227551c3d105909fc9866134bbd7bcf2cf668f
Diff: https://reviews.apache.org/r/55904/diff/3/
Changes: https://reviews.apache.org/r/55904/diff/2-3/
Testing (updated)
-------
Added new unit test in TestSentryStore. Revised unit test in TestFullUpdateInitializer.
Thanks,
Hao Hao
Re: Review Request 55904: SENTRY-1612: HMSFollower should persist
full HMS snapshot into SentryDB if there is not one.
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55904/#review167619
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Kolbasov
On March 2, 2017, 12:03 a.m., Hao Hao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55904/
> -----------------------------------------------------------
>
> (Updated March 2, 2017, 12:03 a.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> After reads a point-in-time full HMSPaths snapshot from HMS and record the corresponding
> notification ID, HMSFollwer should persist it into Sentry DB.
>
> Otherwise if a full snapshot is stored in DB, read the corresponding currentEventID from sentry store.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java f95dd9405bce6f5fb6ec0e0c708a57a3093a1db0
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java ffb07560d7c40fb14cef77950012c2c81fa3fd28
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestFullUpdateInitializer.java 0bb6f665ae51f7dd63a9bee545150a85f42274a5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java 0ca7fe254008b021e86edb42c70fe002eb801930
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b9272bc80ea473685b90f6c30dc8e0b0dff7b9a9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java bdbb0cc3ff639bf2e5c3725e6ebf1cc641c01374
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 0e227551c3d105909fc9866134bbd7bcf2cf668f
>
>
> Diff: https://reviews.apache.org/r/55904/diff/5/
>
>
> Testing
> -------
>
> Added new unit test in TestSentryStore. Revised unit test in TestFullUpdateInitializer.
>
>
> Thanks,
>
> Hao Hao
>
>
Re: Review Request 55904: SENTRY-1612: HMSFollower should persist
full HMS snapshot into SentryDB if there is not one.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55904/
-----------------------------------------------------------
(Updated March 2, 2017, 12:03 a.m.)
Review request for sentry.
Repository: sentry
Description
-------
After reads a point-in-time full HMSPaths snapshot from HMS and record the corresponding
notification ID, HMSFollwer should persist it into Sentry DB.
Otherwise if a full snapshot is stored in DB, read the corresponding currentEventID from sentry store.
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java f95dd9405bce6f5fb6ec0e0c708a57a3093a1db0
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java ffb07560d7c40fb14cef77950012c2c81fa3fd28
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestFullUpdateInitializer.java 0bb6f665ae51f7dd63a9bee545150a85f42274a5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java 0ca7fe254008b021e86edb42c70fe002eb801930
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b9272bc80ea473685b90f6c30dc8e0b0dff7b9a9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java bdbb0cc3ff639bf2e5c3725e6ebf1cc641c01374
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 0e227551c3d105909fc9866134bbd7bcf2cf668f
Diff: https://reviews.apache.org/r/55904/diff/5/
Changes: https://reviews.apache.org/r/55904/diff/4-5/
Testing
-------
Added new unit test in TestSentryStore. Revised unit test in TestFullUpdateInitializer.
Thanks,
Hao Hao
Re: Review Request 55904: SENTRY-1612: HMSFollower should persist
full HMS snapshot into SentryDB if there is not one.
Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55904/
-----------------------------------------------------------
(Updated March 1, 2017, 11:30 p.m.)
Review request for sentry.
Repository: sentry
Description
-------
After reads a point-in-time full HMSPaths snapshot from HMS and record the corresponding
notification ID, HMSFollwer should persist it into Sentry DB.
Otherwise if a full snapshot is stored in DB, read the corresponding currentEventID from sentry store.
Diffs (updated)
-----
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java f95dd9405bce6f5fb6ec0e0c708a57a3093a1db0
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java ffb07560d7c40fb14cef77950012c2c81fa3fd28
sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestFullUpdateInitializer.java 0bb6f665ae51f7dd63a9bee545150a85f42274a5
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java 0ca7fe254008b021e86edb42c70fe002eb801930
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b9272bc80ea473685b90f6c30dc8e0b0dff7b9a9
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java bdbb0cc3ff639bf2e5c3725e6ebf1cc641c01374
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 0e227551c3d105909fc9866134bbd7bcf2cf668f
Diff: https://reviews.apache.org/r/55904/diff/4/
Changes: https://reviews.apache.org/r/55904/diff/3-4/
Testing
-------
Added new unit test in TestSentryStore. Revised unit test in TestFullUpdateInitializer.
Thanks,
Hao Hao
Re: Review Request 55904: SENTRY-1612: HMSFollower should persist
full HMS snapshot into SentryDB if there is not one.
Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55904/#review167587
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java
Lines 340 (patched)
<https://reviews.apache.org/r/55904/#comment239499>
You can check upfront - if pathsUpdate.getPathChanges().isEmpty, return empty map.
Otherwise you can create map with capacity based on pathsUpdate.getPathChanges().size()
This isn't a big deal - just an optimization to consider.
sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java
Lines 165 (patched)
<https://reviews.apache.org/r/55904/#comment239492>
Naming - it isn't a getter.
For this one and a splitter - I've seen the same code in multiple places - can we use some common method for these?
Also, do we need a list or any Iterable or Collection will do?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Line 295 (original), 302 (patched)
<https://reviews.apache.org/r/55904/#comment239497>
Incomplete fraze?
retrieve full HMS snapshot from ... ?
- Alexander Kolbasov
On March 1, 2017, 8:20 p.m., Hao Hao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55904/
> -----------------------------------------------------------
>
> (Updated March 1, 2017, 8:20 p.m.)
>
>
> Review request for sentry.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> After reads a point-in-time full HMSPaths snapshot from HMS and record the corresponding
> notification ID, HMSFollwer should persist it into Sentry DB.
>
> Otherwise if a full snapshot is stored in DB, read the corresponding currentEventID from sentry store.
>
>
> Diffs
> -----
>
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java f95dd9405bce6f5fb6ec0e0c708a57a3093a1db0
> sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java ffb07560d7c40fb14cef77950012c2c81fa3fd28
> sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestFullUpdateInitializer.java 0bb6f665ae51f7dd63a9bee545150a85f42274a5
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPathChange.java 0ca7fe254008b021e86edb42c70fe002eb801930
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b9272bc80ea473685b90f6c30dc8e0b0dff7b9a9
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java bdbb0cc3ff639bf2e5c3725e6ebf1cc641c01374
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java 0e227551c3d105909fc9866134bbd7bcf2cf668f
>
>
> Diff: https://reviews.apache.org/r/55904/diff/3/
>
>
> Testing
> -------
>
> Added new unit test in TestSentryStore. Revised unit test in TestFullUpdateInitializer.
>
>
> Thanks,
>
> Hao Hao
>
>