You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2014/08/04 20:02:45 UTC
Review Request 24243: DB schema for the job update store.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/
-----------------------------------------------------------
Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
Bugs: AURORA-612
https://issues.apache.org/jira/browse/AURORA-612
Repository: aurora
Description
-------
DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
Diffs
-----
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
Diff: https://reviews.apache.org/r/24243/diff/
Testing
-------
Thanks,
Maxim Khutornenko
Re: Review Request 24243: DB schema for the job update store.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49505
-----------------------------------------------------------
Ship it!
+1 from me after the double-negative is removed
- Kevin Sweeney
On Aug. 4, 2014, 1:28 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 1:28 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Kevin Sweeney <ke...@apache.org>.
> On Aug. 4, 2014, 1:31 p.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 111
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line111>
> >
> > Will the interface return RangeSet<Integer>? Maybe a more compact form could be used on disk as well.
>
> Maxim Khutornenko wrote:
> Most likely. I can't see a more compact way on the SQL side though (given the existing H2 type set) without introducing more tables.
>
> Bill Farner wrote:
> Introducing more tables is indeed the trade-off that should be discussed. I suggested to Maxim that we start with ARRAY and fall back to relations if we can't get it to work smoothly with mybatis.
Since they both have total orderings that fit our use case I can see sticking to unix timestamps for simplicity. ISO 8601 timestamps are easier to read on the wire than unix ones though, but this could be handled by a presentation layer.
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49492
-----------------------------------------------------------
On Aug. 4, 2014, 1:28 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 1:28 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Aug. 4, 2014, 8:31 p.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, lines 101-102
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line101>
> >
> > Is there a reason not to use TIMESTAMP [1] types that will be translated to a JDBC Timestamp [2] type here?
> >
> > [1] http://www.h2database.com/html/datatypes.html#timestamp_type
> > [2] http://docs.oracle.com/javase/7/docs/api/java/sql/Timestamp.html
The most likely use case for these would be the scheduler UI. I doubt java side would benefit from anything more complex than Long here (aside from extra validation). Also, the precedent is already set in locks table.
> On Aug. 4, 2014, 8:31 p.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 111
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line111>
> >
> > Will the interface return RangeSet<Integer>? Maybe a more compact form could be used on disk as well.
Most likely. I can't see a more compact way on the SQL side though (given the existing H2 type set) without introducing more tables.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49492
-----------------------------------------------------------
On Aug. 4, 2014, 8:28 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 8:28 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Bill Farner <wf...@apache.org>.
> On Aug. 4, 2014, 8:31 p.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, lines 101-102
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line101>
> >
> > Is there a reason not to use TIMESTAMP [1] types that will be translated to a JDBC Timestamp [2] type here?
> >
> > [1] http://www.h2database.com/html/datatypes.html#timestamp_type
> > [2] http://docs.oracle.com/javase/7/docs/api/java/sql/Timestamp.html
>
> Maxim Khutornenko wrote:
> The most likely use case for these would be the scheduler UI. I doubt java side would benefit from anything more complex than Long here (aside from extra validation). Also, the precedent is already set in locks table.
I think exploring this is TODO-worthy, since i would like to see us jump to TIMESTAMP in one fell swoop.
> On Aug. 4, 2014, 8:31 p.m., Kevin Sweeney wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 111
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line111>
> >
> > Will the interface return RangeSet<Integer>? Maybe a more compact form could be used on disk as well.
>
> Maxim Khutornenko wrote:
> Most likely. I can't see a more compact way on the SQL side though (given the existing H2 type set) without introducing more tables.
Introducing more tables is indeed the trade-off that should be discussed. I suggested to Maxim that we start with ARRAY and fall back to relations if we can't get it to work smoothly with mybatis.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49492
-----------------------------------------------------------
On Aug. 4, 2014, 8:28 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 8:28 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49492
-----------------------------------------------------------
partial review on v1, will deliver full with v2
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86567>
Is there a reason not to use TIMESTAMP [1] types that will be translated to a JDBC Timestamp [2] type here?
[1] http://www.h2database.com/html/datatypes.html#timestamp_type
[2] http://docs.oracle.com/javase/7/docs/api/java/sql/Timestamp.html
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86577>
Will the interface return RangeSet<Integer>? Maybe a more compact form could be used on disk as well.
- Kevin Sweeney
On Aug. 4, 2014, 1:28 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 1:28 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49511
-----------------------------------------------------------
FlagSchemaChanges is going to fail if this is committed as-is.
- Kevin Sweeney
On Aug. 4, 2014, 2:04 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 2:04 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
> src/main/thrift/org/apache/aurora/gen/api.thrift 595c8dfc3d28c2bfa92ae2226a28a8519d75b8c4
> src/test/python/apache/aurora/client/api/test_scheduler_client.py e1f1a95d1757d584056fe6be9c98049444f6a75b
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49523
-----------------------------------------------------------
Ship it!
Ship It!
- David McLaughlin
On Aug. 4, 2014, 10:06 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 10:06 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
> src/main/thrift/org/apache/aurora/gen/api.thrift 595c8dfc3d28c2bfa92ae2226a28a8519d75b8c4
> src/test/python/apache/aurora/client/api/test_scheduler_client.py e1f1a95d1757d584056fe6be9c98049444f6a75b
> src/test/resources/org/apache/aurora/gen/api.thrift.md5 3245817b009849ed49d2e259c04a1a3e1710a440
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/
-----------------------------------------------------------
(Updated Aug. 4, 2014, 10:06 p.m.)
Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
Changes
-------
CR comments.
Bugs: AURORA-612
https://issues.apache.org/jira/browse/AURORA-612
Repository: aurora
Description
-------
DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
Diffs (updated)
-----
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
src/main/thrift/org/apache/aurora/gen/api.thrift 595c8dfc3d28c2bfa92ae2226a28a8519d75b8c4
src/test/python/apache/aurora/client/api/test_scheduler_client.py e1f1a95d1757d584056fe6be9c98049444f6a75b
src/test/resources/org/apache/aurora/gen/api.thrift.md5 3245817b009849ed49d2e259c04a1a3e1710a440
Diff: https://reviews.apache.org/r/24243/diff/
Testing
-------
Thanks,
Maxim Khutornenko
Re: Review Request 24243: DB schema for the job update store.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Aug. 4, 2014, 9:41 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 98
> > <https://reviews.apache.org/r/24243/diff/4/?file=650982#file650982line98>
> >
> > Strongly consider dropping update_id to let the IDENTITY stand alone.
If we were not returning the ID within the same call I would definitely go for the IDENTITY as ID. However, after spending hours trying to make MERGE statement work correctly with the auto-generated ID I had to give up.
Also, the updateID appears consistent with taskID where the value is generated before accessing the store.
> On Aug. 4, 2014, 9:41 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 101
> > <https://reviews.apache.org/r/24243/diff/4/?file=650982#file650982line101>
> >
> > Feel free to pull the comment up to the top, to signify that we want to change all to TIMESTAMP.
Done.
> On Aug. 4, 2014, 9:41 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 117
> > <https://reviews.apache.org/r/24243/diff/4/?file=650982#file650982line117>
> >
> > Feel free to punt in this review, but BLOB might be preferable here.
Thanks for the idea. Will definitely consider when dealing with TaskConfig serialization.
> On Aug. 4, 2014, 9:41 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 562
> > <https://reviews.apache.org/r/24243/diff/4/?file=650983#file650983line562>
> >
> > s/True/true/, which is the keyword thrift uses.
Done.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49510
-----------------------------------------------------------
On Aug. 4, 2014, 9:04 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 9:04 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
> src/main/thrift/org/apache/aurora/gen/api.thrift 595c8dfc3d28c2bfa92ae2226a28a8519d75b8c4
> src/test/python/apache/aurora/client/api/test_scheduler_client.py e1f1a95d1757d584056fe6be9c98049444f6a75b
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49510
-----------------------------------------------------------
Ship it!
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86588>
Strongly consider dropping update_id to let the IDENTITY stand alone.
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86587>
Feel free to pull the comment up to the top, to signify that we want to change all to TIMESTAMP.
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86593>
Feel free to punt in this review, but BLOB might be preferable here.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/24243/#comment86585>
s/True/true/, which is the keyword thrift uses.
src/test/python/apache/aurora/client/api/test_scheduler_client.py
<https://reviews.apache.org/r/24243/#comment86586>
doh, thanks
- Bill Farner
On Aug. 4, 2014, 9:04 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 9:04 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
> src/main/thrift/org/apache/aurora/gen/api.thrift 595c8dfc3d28c2bfa92ae2226a28a8519d75b8c4
> src/test/python/apache/aurora/client/api/test_scheduler_client.py e1f1a95d1757d584056fe6be9c98049444f6a75b
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/
-----------------------------------------------------------
(Updated Aug. 4, 2014, 9:04 p.m.)
Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
Changes
-------
CR comments and isort fixes.
Bugs: AURORA-612
https://issues.apache.org/jira/browse/AURORA-612
Repository: aurora
Description
-------
DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
Diffs (updated)
-----
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
src/main/thrift/org/apache/aurora/gen/api.thrift 595c8dfc3d28c2bfa92ae2226a28a8519d75b8c4
src/test/python/apache/aurora/client/api/test_scheduler_client.py e1f1a95d1757d584056fe6be9c98049444f6a75b
Diff: https://reviews.apache.org/r/24243/diff/
Testing
-------
Thanks,
Maxim Khutornenko
Re: Review Request 24243: DB schema for the job update store.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/
-----------------------------------------------------------
(Updated Aug. 4, 2014, 8:59 p.m.)
Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
Changes
-------
Added a TODO for TIMESTAMP.
Bugs: AURORA-612
https://issues.apache.org/jira/browse/AURORA-612
Repository: aurora
Description
-------
DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
Diffs (updated)
-----
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
Diff: https://reviews.apache.org/r/24243/diff/
Testing
-------
Thanks,
Maxim Khutornenko
Re: Review Request 24243: DB schema for the job update store.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/
-----------------------------------------------------------
(Updated Aug. 4, 2014, 8:28 p.m.)
Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
Changes
-------
CR comments.
Bugs: AURORA-612
https://issues.apache.org/jira/browse/AURORA-612
Repository: aurora
Description
-------
DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
Diffs (updated)
-----
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
Diff: https://reviews.apache.org/r/24243/diff/
Testing
-------
Thanks,
Maxim Khutornenko
Re: Review Request 24243: DB schema for the job update store.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On Aug. 4, 2014, 7:27 p.m., Bill Farner wrote:
> >
Current design assumes H2 ARRAY type is supported in MyBatis.
> On Aug. 4, 2014, 7:27 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 100
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line100>
> >
> > REFERENCES update_statuses(id)
Done.
> On Aug. 4, 2014, 7:27 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 80
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line80>
> >
> > Please prefix tables with job_ where appropriate.
Done.
> On Aug. 4, 2014, 7:27 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 107
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line107>
> >
> > What's the motivation behind separating updates, update_configs, and update_settings? They seem tightly-coupled enough that they might benefit from being combined.
The update_settings can be merged with updates (Done).
The update_configs have to be separate as updates will have multiple TaskConfig records covering different instance ranges in "old" config domain. The "new" task config will also be stored in this table.
> On Aug. 4, 2014, 7:27 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 112
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line112>
> >
> > What's this for?
To separate new from old task configs.
> On Aug. 4, 2014, 7:27 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 123
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line123>
> >
> > I'm an anti-fan of flags that say "do not do X", as they can lead to confusing double-negatives. Consider s/do_not_//
This maps into the do_not_rollback_on_failure flag in thrift UpdateSettings. The idea here is that a default False value (when not initialized) would not break the default current behavior to rollback. Do you suggest we change it in thrift too?
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49487
-----------------------------------------------------------
On Aug. 4, 2014, 6:02 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 6:02 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Bill Farner <wf...@apache.org>.
> On Aug. 4, 2014, 7:27 p.m., Bill Farner wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 123
> > <https://reviews.apache.org/r/24243/diff/1/?file=650526#file650526line123>
> >
> > I'm an anti-fan of flags that say "do not do X", as they can lead to confusing double-negatives. Consider s/do_not_//
>
> Maxim Khutornenko wrote:
> This maps into the do_not_rollback_on_failure flag in thrift UpdateSettings. The idea here is that a default False value (when not initialized) would not break the default current behavior to rollback. Do you suggest we change it in thrift too?
Yes, change in thrift as well would be nice.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49487
-----------------------------------------------------------
On Aug. 4, 2014, 8:59 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 8:59 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>
Re: Review Request 24243: DB schema for the job update store.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24243/#review49487
-----------------------------------------------------------
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86555>
Please prefix tables with job_ where appropriate.
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86561>
REFERENCES update_statuses(id)
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86564>
What's the motivation behind separating updates, update_configs, and update_settings? They seem tightly-coupled enough that they might benefit from being combined.
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86562>
What's this for?
src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
<https://reviews.apache.org/r/24243/#comment86563>
I'm an anti-fan of flags that say "do not do X", as they can lead to confusing double-negatives. Consider s/do_not_//
- Bill Farner
On Aug. 4, 2014, 6:02 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24243/
> -----------------------------------------------------------
>
> (Updated Aug. 4, 2014, 6:02 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
>
>
> Bugs: AURORA-612
> https://issues.apache.org/jira/browse/AURORA-612
>
>
> Repository: aurora
>
>
> Description
> -------
>
> DB tables for the job update store. Sending out early to solicit feedback before moving to mappers.
>
>
> Diffs
> -----
>
> src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 5358b45102f53eea97a1ca709ba9375daa91a3ef
>
> Diff: https://reviews.apache.org/r/24243/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Maxim Khutornenko
>
>