You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li via Review Board <no...@reviews.apache.org> on 2020/07/23 20:55:21 UTC
Review Request 72704: SENTRY-2422: HMS synchronization is causing
multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72704/
-----------------------------------------------------------
Review request for sentry and kalyan kumar kalvagadda.
Bugs: sentry-2422
https://issues.apache.org/jira/browse/sentry-2422
Repository: sentry
Description
-------
before saving notification ID, check to make sure only save it if it is larger than the values of existing entries. We only track and use max(Notification ID). So no need to persist duplicated values.
Diffs
-----
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 980c8ad
sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java fd14963
Diff: https://reviews.apache.org/r/72704/diff/1/
Testing
-------
add new unit tests, and they pass
Thanks,
Na Li
Re: Review Request 72704: SENTRY-2422: HMS synchronization is causing
multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID
Posted by Na Li via Review Board <no...@reviews.apache.org>.
> On July 24, 2020, 4 p.m., kalyan kumar kalvagadda wrote:
> > Lina,
> >
> > Do you why are we ssting this issue? Notication should be processed by one sentry lender and there is only one thread the processes the notfications and updates the database.
Kalyan,
This is deployed on cloud. Each cluster has its own Sentry server instance, and all sentry servers point to the same Sentry DB. Therefore, the leader in each cluster syncs HMS notifications, and save into the same table at the same database. That makes the number of duplicated entries to increase very fast. That is why we need to avoid the duplicates.
- Na
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72704/#review221362
-----------------------------------------------------------
On July 23, 2020, 8:55 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72704/
> -----------------------------------------------------------
>
> (Updated July 23, 2020, 8:55 p.m.)
>
>
> Review request for sentry and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2422
> https://issues.apache.org/jira/browse/sentry-2422
>
>
> Repository: sentry
>
>
> Description
> -------
>
> before saving notification ID, check to make sure only save it if it is larger than the values of existing entries. We only track and use max(Notification ID). So no need to persist duplicated values.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 980c8ad
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java fd14963
>
>
> Diff: https://reviews.apache.org/r/72704/diff/1/
>
>
> Testing
> -------
>
> add new unit tests, and they pass
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 72704: SENTRY-2422: HMS synchronization is causing
multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
> On July 24, 2020, 4 p.m., kalyan kumar kalvagadda wrote:
> > Lina,
> >
> > Do you why are we ssting this issue? Notication should be processed by one sentry lender and there is only one thread the processes the notfications and updates the database.
>
> Na Li wrote:
> Kalyan,
>
> This is deployed on cloud. Each cluster has its own Sentry server instance, and all sentry servers point to the same Sentry DB. Therefore, the leader in each cluster syncs HMS notifications, and save into the same table at the same database. That makes the number of duplicated entries to increase very fast. That is why we need to avoid the duplicates.
Can you explain the deployment?
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72704/#review221362
-----------------------------------------------------------
On July 23, 2020, 8:55 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72704/
> -----------------------------------------------------------
>
> (Updated July 23, 2020, 8:55 p.m.)
>
>
> Review request for sentry and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2422
> https://issues.apache.org/jira/browse/sentry-2422
>
>
> Repository: sentry
>
>
> Description
> -------
>
> before saving notification ID, check to make sure only save it if it is larger than the values of existing entries. We only track and use max(Notification ID). So no need to persist duplicated values.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 980c8ad
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java fd14963
>
>
> Diff: https://reviews.apache.org/r/72704/diff/1/
>
>
> Testing
> -------
>
> add new unit tests, and they pass
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 72704: SENTRY-2422: HMS synchronization is causing
multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72704/#review221362
-----------------------------------------------------------
Lina,
Do you why are we ssting this issue? Notication should be processed by one sentry lender and there is only one thread the processes the notfications and updates the database.
- kalyan kumar kalvagadda
On July 23, 2020, 8:55 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72704/
> -----------------------------------------------------------
>
> (Updated July 23, 2020, 8:55 p.m.)
>
>
> Review request for sentry and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2422
> https://issues.apache.org/jira/browse/sentry-2422
>
>
> Repository: sentry
>
>
> Description
> -------
>
> before saving notification ID, check to make sure only save it if it is larger than the values of existing entries. We only track and use max(Notification ID). So no need to persist duplicated values.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 980c8ad
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java fd14963
>
>
> Diff: https://reviews.apache.org/r/72704/diff/1/
>
>
> Testing
> -------
>
> add new unit tests, and they pass
>
>
> Thanks,
>
> Na Li
>
>
Re: Review Request 72704: SENTRY-2422: HMS synchronization is causing
multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID
Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72704/#review221367
-----------------------------------------------------------
Ship it!
Change looks good, you need to understand that this reduces the chance of having dplicate entries but does not guarentee it.
- kalyan kumar kalvagadda
On July 23, 2020, 8:55 p.m., Na Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72704/
> -----------------------------------------------------------
>
> (Updated July 23, 2020, 8:55 p.m.)
>
>
> Review request for sentry and kalyan kumar kalvagadda.
>
>
> Bugs: sentry-2422
> https://issues.apache.org/jira/browse/sentry-2422
>
>
> Repository: sentry
>
>
> Description
> -------
>
> before saving notification ID, check to make sure only save it if it is larger than the values of existing entries. We only track and use max(Notification ID). So no need to persist duplicated values.
>
>
> Diffs
> -----
>
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 980c8ad
> sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java fd14963
>
>
> Diff: https://reviews.apache.org/r/72704/diff/1/
>
>
> Testing
> -------
>
> add new unit tests, and they pass
>
>
> Thanks,
>
> Na Li
>
>