You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Alexander Kolbasov <ak...@gmail.com> on 2016/11/19 01:45:43 UTC

Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/
-----------------------------------------------------------

Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya Tirukkovalur, and Vadim Spector.


Repository: sentry


Description
-------

SENTRY-1534: Oracle supports serializable instead of repeatable-read


Diffs
-----

  sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ec47909ab3098a890019bc19179786a2c5d7d87c 
  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 742798dca6b3ad8c9b9eee1eea5380001d1d0cd9 

Diff: https://reviews.apache.org/r/53920/diff/


Testing
-------


Thanks,

Alexander Kolbasov


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On Nov. 23, 2016, 1:38 a.m., Hao Hao wrote:
> > sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java, line 149
> > <https://reviews.apache.org/r/53920/diff/1/?file=1567316#file1567316line149>
> >
> >     Could you explain a bit what is autoCreateAll do? And why should we remove it in this case? The default would be false?

When sentry starts it loggs:

Property datanucleus.schema.autoCreateAll unknown - will be ignored
         datanucleus.schema.autoCreateAll

so our version of datanucleus doesn't understand it. 

That said it is a valid property according to the current docs, so we may as well keep it. I'll revert this change.


> On Nov. 23, 2016, 1:38 a.m., Hao Hao wrote:
> > sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java, line 154
> > <https://reviews.apache.org/r/53920/diff/1/?file=1567316#file1567316line154>
> >
> >     Why do we need datanucleus cache l2 here?

The correct setting is datanucleus.cache.level2.type set to none which is done above.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/#review156683
-----------------------------------------------------------


On Nov. 19, 2016, 1:45 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53920/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2016, 1:45 a.m.)
> 
> 
> Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya Tirukkovalur, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1534: Oracle supports serializable instead of repeatable-read
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ec47909ab3098a890019bc19179786a2c5d7d87c 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 742798dca6b3ad8c9b9eee1eea5380001d1d0cd9 
> 
> Diff: https://reviews.apache.org/r/53920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Hao Hao <ha...@cloudera.com>.

> On Nov. 23, 2016, 1:38 a.m., Hao Hao wrote:
> > sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java, line 149
> > <https://reviews.apache.org/r/53920/diff/1/?file=1567316#file1567316line149>
> >
> >     Could you explain a bit what is autoCreateAll do? And why should we remove it in this case? The default would be false?
> 
> Alexander Kolbasov wrote:
>     When sentry starts it loggs:
>     
>     Property datanucleus.schema.autoCreateAll unknown - will be ignored
>              datanucleus.schema.autoCreateAll
>     
>     so our version of datanucleus doesn't understand it. 
>     
>     That said it is a valid property according to the current docs, so we may as well keep it. I'll revert this change.

From the docs, it does look like a valid one. Is it because the particular version we use does not support it? If so, we may want to replace it with the corresponding property in the version we are using?


- Hao


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/#review156683
-----------------------------------------------------------


On Nov. 19, 2016, 1:45 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53920/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2016, 1:45 a.m.)
> 
> 
> Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya Tirukkovalur, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1534: Oracle supports serializable instead of repeatable-read
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ec47909ab3098a890019bc19179786a2c5d7d87c 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 742798dca6b3ad8c9b9eee1eea5380001d1d0cd9 
> 
> Diff: https://reviews.apache.org/r/53920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/#review156683
-----------------------------------------------------------




sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 
<https://reviews.apache.org/r/53920/#comment226900>

    Could you explain a bit what is autoCreateAll do? And why should we remove it in this case? The default would be false?



sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 
<https://reviews.apache.org/r/53920/#comment226901>

    Why do we need datanucleus cache l2 here?


- Hao Hao


On Nov. 19, 2016, 1:45 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53920/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2016, 1:45 a.m.)
> 
> 
> Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya Tirukkovalur, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1534: Oracle supports serializable instead of repeatable-read
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ec47909ab3098a890019bc19179786a2c5d7d87c 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 742798dca6b3ad8c9b9eee1eea5380001d1d0cd9 
> 
> Diff: https://reviews.apache.org/r/53920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On Jan. 5, 2017, 9:33 p.m., kalyan kumar kalvagadda wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 154
> > <https://reviews.apache.org/r/53920/diff/3/?file=1597183#file1597183line154>
> >
> >     Trying to understand your statement about oracle not supporting repeatable-read. How did you conclude it? I see what you are saying in some websites but the oracle documentation says they support it.

Take a look at http://docs.oracle.com/database/121/ADFNS/adfns_sqlproc.htm#ADFNS99871, for example. In particular, it says that you get 
REPEATABLE READ if you set the transaction isolation level to SERIALIZABLE.

The problem is that an attempt to set isolation level to repeatable read is rejected by Oracle and you get an error.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/#review160634
-----------------------------------------------------------


On Jan. 5, 2017, 9:04 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53920/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 9:04 a.m.)
> 
> 
> Review request for sentry, Colin Ma, Hao Hao, kalyan kumar kalvagadda, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1534
>     https://issues.apache.org/jira/browse/SENTRY-1534
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1534: Oracle supports serializable instead of repeatable-read
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 919fdaf11c36b412211176ede3c98170a2e34235 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f83d72160e1c44442d3d1ec1b870ea4b4d5dda1a 
> 
> Diff: https://reviews.apache.org/r/53920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/#review160634
-----------------------------------------------------------




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 154)
<https://reviews.apache.org/r/53920/#comment231770>

    Trying to understand your statement about oracle not supporting repeatable-read. How did you conclude it? I see what you are saying in some websites but the oracle documentation says they support it.


- kalyan kumar kalvagadda


On Jan. 5, 2017, 9:04 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53920/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 9:04 a.m.)
> 
> 
> Review request for sentry, Colin Ma, Hao Hao, kalyan kumar kalvagadda, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1534
>     https://issues.apache.org/jira/browse/SENTRY-1534
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1534: Oracle supports serializable instead of repeatable-read
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 919fdaf11c36b412211176ede3c98170a2e34235 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f83d72160e1c44442d3d1ec1b870ea4b4d5dda1a 
> 
> Diff: https://reviews.apache.org/r/53920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/#review160957
-----------------------------------------------------------


Ship it!




Ship It!

- Hao Hao


On Jan. 5, 2017, 9:04 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53920/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 9:04 a.m.)
> 
> 
> Review request for sentry, Colin Ma, Hao Hao, kalyan kumar kalvagadda, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1534
>     https://issues.apache.org/jira/browse/SENTRY-1534
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1534: Oracle supports serializable instead of repeatable-read
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 919fdaf11c36b412211176ede3c98170a2e34235 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f83d72160e1c44442d3d1ec1b870ea4b4d5dda1a 
> 
> Diff: https://reviews.apache.org/r/53920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/
-----------------------------------------------------------

(Updated Jan. 5, 2017, 9:04 a.m.)


Review request for sentry, Colin Ma, Hao Hao, kalyan kumar kalvagadda, Vamsee Yarlagadda, and Vadim Spector.


Changes
-------

Removed all changes not directly related to the fix.


Bugs: SENTRY-1534
    https://issues.apache.org/jira/browse/SENTRY-1534


Repository: sentry


Description
-------

SENTRY-1534: Oracle supports serializable instead of repeatable-read


Diffs (updated)
-----

  sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 919fdaf11c36b412211176ede3c98170a2e34235 
  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f83d72160e1c44442d3d1ec1b870ea4b4d5dda1a 

Diff: https://reviews.apache.org/r/53920/diff/


Testing
-------


Thanks,

Alexander Kolbasov


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Alexander Kolbasov <ak...@gmail.com>.

> On Nov. 30, 2016, 11:55 p.m., Hao Hao wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, line 167
> > <https://reviews.apache.org/r/53920/diff/2/?file=1571678#file1571678line167>
> >
> >     Does "serializable" in Oracle mean the same thing as "repeatable" in other databases?

It isn't exactly the same. Oracle provides just two isolation levels - read-committed and serializable. Serializable is stronger than repeatable-read and will also protect us from updating the same row from two transactions at the same time.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/#review157501
-----------------------------------------------------------


On Jan. 5, 2017, 9:04 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53920/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 9:04 a.m.)
> 
> 
> Review request for sentry, Colin Ma, Hao Hao, kalyan kumar kalvagadda, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1534
>     https://issues.apache.org/jira/browse/SENTRY-1534
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1534: Oracle supports serializable instead of repeatable-read
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 919fdaf11c36b412211176ede3c98170a2e34235 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java f83d72160e1c44442d3d1ec1b870ea4b4d5dda1a 
> 
> Diff: https://reviews.apache.org/r/53920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Hao Hao <ha...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/#review157501
-----------------------------------------------------------




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java (line 167)
<https://reviews.apache.org/r/53920/#comment228107>

    Does "serializable" in Oracle mean the same thing as "repeatable" in other databases?


- Hao Hao


On Nov. 29, 2016, 6:29 a.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53920/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2016, 6:29 a.m.)
> 
> 
> Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya Tirukkovalur, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1534: Oracle supports serializable instead of repeatable-read
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 919fdaf11c36b412211176ede3c98170a2e34235 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 742798dca6b3ad8c9b9eee1eea5380001d1d0cd9 
> 
> Diff: https://reviews.apache.org/r/53920/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>


Re: Review Request 53920: SENTRY-1534: Oracle supports serializable instead of repeatable-read

Posted by Alexander Kolbasov <ak...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53920/
-----------------------------------------------------------

(Updated Nov. 29, 2016, 6:29 a.m.)


Review request for sentry, Colm O hEigeartaigh, Colin Ma, Hao Hao, Sravya Tirukkovalur, and Vadim Spector.


Changes
-------

Restored autoCreateAll


Repository: sentry


Description
-------

SENTRY-1534: Oracle supports serializable instead of repeatable-read


Diffs (updated)
-----

  sentry-service/sentry-service-common/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java 919fdaf11c36b412211176ede3c98170a2e34235 
  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 742798dca6b3ad8c9b9eee1eea5380001d1d0cd9 

Diff: https://reviews.apache.org/r/53920/diff/


Testing
-------


Thanks,

Alexander Kolbasov