Re: Review Request 72704: SENTRY-2422: HMS synchronization is causing multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID

2020-07-24 Thread kalyan kumar kalvagadda via Review Board


> 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

2020-07-24 Thread kalyan kumar kalvagadda via Review Board

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



Re: Review Request 72704: SENTRY-2422: HMS synchronization is causing multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID

2020-07-24 Thread Na Li via Review Board


> 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

2020-07-24 Thread kalyan kumar kalvagadda via Review Board

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



Review Request 72704: SENTRY-2422: HMS synchronization is causing multiple entries of the same ID in SENTRY_HMS_NOTIFICATION_ID

2020-07-23 Thread Na Li via Review Board

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