You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2016/01/21 21:24:04 UTC
Review Request 42613: Enable READ COMMITTED transaction isolation.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/
-----------------------------------------------------------
Review request for Aurora, John Sirois and Zameer Manji.
Bugs: AURORA-1580
https://issues.apache.org/jira/browse/AURORA-1580
Repository: aurora
Description
-------
Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
Diff: https://reviews.apache.org/r/42613/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115688
-----------------------------------------------------------
Ship it!
Ship It!
- Maxim Khutornenko
On Jan. 21, 2016, 8:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 8:25 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Jan. 21, 2016, 8:28 p.m., John Sirois wrote:
> > IIUC the MVCC capabilities in 1.4+ should prevent table locks on all but DDL stuff.
>
> John Sirois wrote:
> Hmm - more to the story: "If MVCC is enabled, changing the lock mode (LOCK_MODE) has no effect." ... Maybe turning on MVCC is worth some perf tests or have you already been down that road?
I suggest a cautios approach to enabling MVCC as our previous experience showed unacceptable performance with any meaningful amounts of data.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115684
-----------------------------------------------------------
On Jan. 21, 2016, 8:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 8:25 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by John Sirois <js...@apache.org>.
> On Jan. 21, 2016, 1:28 p.m., John Sirois wrote:
> > IIUC the MVCC capabilities in 1.4+ should prevent table locks on all but DDL stuff.
>
> John Sirois wrote:
> Hmm - more to the story: "If MVCC is enabled, changing the lock mode (LOCK_MODE) has no effect." ... Maybe turning on MVCC is worth some perf tests or have you already been down that road?
>
> Maxim Khutornenko wrote:
> I suggest a cautios approach to enabling MVCC as our previous experience showed unacceptable performance with any meaningful amounts of data.
>
> Bill Farner wrote:
> Bigger caution for me is that MVCC is considered beta in H2.
The beta bit is not at all clear to me. If I read like a lawyer, I _think_ I agree, but I don;t find this all that clear!:
> The MVCC mode is enabled by default in version 1.4.x, with the default MVStore storage engine. MVCC is disabled by default when using the PageStore storage engine (which is the default in version 1.3.x). The following applies when using the PageStore storage engine: The MVCC feature is not fully tested yet.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115684
-----------------------------------------------------------
On Jan. 21, 2016, 1:40 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 1:40 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by Bill Farner <wf...@apache.org>.
> On Jan. 21, 2016, 12:28 p.m., John Sirois wrote:
> > IIUC the MVCC capabilities in 1.4+ should prevent table locks on all but DDL stuff.
>
> John Sirois wrote:
> Hmm - more to the story: "If MVCC is enabled, changing the lock mode (LOCK_MODE) has no effect." ... Maybe turning on MVCC is worth some perf tests or have you already been down that road?
>
> Maxim Khutornenko wrote:
> I suggest a cautios approach to enabling MVCC as our previous experience showed unacceptable performance with any meaningful amounts of data.
Bigger caution for me is that MVCC is considered beta in H2.
> On Jan. 21, 2016, 12:28 p.m., John Sirois wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 115
> > <https://reviews.apache.org/r/42613/diff/1/?file=1204582#file1204582line115>
> >
> > Please fix the comment and add a pointer to http://www.h2database.com/html/advanced.html?highlight=LOCK_MODE&search=LOCK#transaction_isolation since "3" is so cryptic and folks - like me just now - will want to double check.
Agreed, fixed.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115684
-----------------------------------------------------------
On Jan. 21, 2016, 12:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 12:25 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by John Sirois <js...@apache.org>.
> On Jan. 21, 2016, 1:28 p.m., John Sirois wrote:
> > IIUC the MVCC capabilities in 1.4+ should prevent table locks on all but DDL stuff.
Hmm - more to the story: "If MVCC is enabled, changing the lock mode (LOCK_MODE) has no effect." ... Maybe turning on MVCC is worth some perf tests or have you already been down that road?
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115684
-----------------------------------------------------------
On Jan. 21, 2016, 1:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 1:25 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115684
-----------------------------------------------------------
Ship it!
IIUC the MVCC capabilities in 1.4+ should prevent table locks on all but DDL stuff.
src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java (line 115)
<https://reviews.apache.org/r/42613/#comment176700>
Please fix the comment and add a pointer to http://www.h2database.com/html/advanced.html?highlight=LOCK_MODE&search=LOCK#transaction_isolation since "3" is so cryptic and folks - like me just now - will want to double check.
- John Sirois
On Jan. 21, 2016, 1:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 1:25 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by John Sirois <js...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115693
-----------------------------------------------------------
Ship it!
Ship It!
- John Sirois
On Jan. 21, 2016, 1:40 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 1:40 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115695
-----------------------------------------------------------
Ship it!
Master (8d3fb24) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Jan. 21, 2016, 8:40 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 8:40 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/#review115705
-----------------------------------------------------------
Ship it!
Ship It!
- Zameer Manji
On Jan. 21, 2016, 12:40 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42613/
> -----------------------------------------------------------
>
> (Updated Jan. 21, 2016, 12:40 p.m.)
>
>
> Review request for Aurora, John Sirois and Zameer Manji.
>
>
> Bugs: AURORA-1580
> https://issues.apache.org/jira/browse/AURORA-1580
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
>
> I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
>
> Diff: https://reviews.apache.org/r/42613/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/
-----------------------------------------------------------
(Updated Jan. 21, 2016, 12:40 p.m.)
Review request for Aurora, John Sirois and Zameer Manji.
Bugs: AURORA-1580
https://issues.apache.org/jira/browse/AURORA-1580
Repository: aurora
Description
-------
Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
Diff: https://reviews.apache.org/r/42613/diff/
Testing
-------
Thanks,
Bill Farner
Re: Review Request 42613: Enable READ COMMITTED transaction isolation.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42613/
-----------------------------------------------------------
(Updated Jan. 21, 2016, 12:25 p.m.)
Review request for Aurora, John Sirois and Zameer Manji.
Bugs: AURORA-1580
https://issues.apache.org/jira/browse/AURORA-1580
Repository: aurora
Description (updated)
-------
Proposing this as the minimal fix for AURORA-1580. I think it's wise, however, to look towards the alternative approach i described in that ticket for simplicity and performance gains.
I'd like to note that after this change, it will be important to treat long-running transactions carefully. It will be important to avoid transactions block others by writing (locking a table) and subsequently doing other lengthy work.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2b3ee7bf6f7801c140f921b25f78daf6d320098a
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e689cfeda39e1f065ed29c61a23376512df8f3aa
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 3dba286556ab6ea34b7fdd38508a164c902d9d17
Diff: https://reviews.apache.org/r/42613/diff/
Testing
-------
Thanks,
Bill Farner