Re: Review Request 64288: Add a SQL persistence implementation

2018-01-20 Thread Stephan Erb
> On Dec. 12, 2017, 11:15 p.m., Stephan Erb wrote: > > The code looks fine and reasonable to me. I would still recommend proper > > scale testing though. > > > > At my company, we operate at a small scale and the Mesos replicated log > > works still well for us. Operating a HA MySQL or

Re: Review Request 64288: Add a SQL persistence implementation

2018-01-04 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review194823 --- Ship it! Master (8e5e08e) is green with this patch.

Re: Review Request 64288: Add a SQL persistence implementation

2018-01-04 Thread Mohit Jaggi
> On Dec. 12, 2017, 10:15 p.m., Stephan Erb wrote: > > The code looks fine and reasonable to me. I would still recommend proper > > scale testing though. > > > > At my company, we operate at a small scale and the Mesos replicated log > > works still well for us. Operating a HA MySQL or

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-12 Thread Bill Farner
> On Dec. 12, 2017, 2:15 p.m., Stephan Erb wrote: > > src/main/resources/org/apache/aurora/scheduler/storage/sql/schema.sql > > Lines 4-8 (patched) > > > > > > Just to check we stay within the comfort zone of a DBMS:

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-12 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review193234 --- Ship it! The code looks fine and reasonable to me. I would

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-07 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review193160 --- Ship it! Master (6c897e5) is green with this patch.

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-07 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/ --- (Updated Dec. 7, 2017, 10:27 a.m.) Review request for Aurora, David

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review192808 --- Ship it! Master (a0628ef) is green with this patch.

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review192756 --- Ship it! Master (a0628ef) is green with this patch.

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/ --- (Updated Dec. 4, 2017, 12:09 p.m.) Review request for Aurora, David

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Bill Farner
> On Dec. 4, 2017, 10:40 a.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/storage/sql/SqlPersistence.java > > Lines 73 (patched) > > > > > > Can this just be `DataSource dataSource` since we're

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Bill Farner
> On Dec. 3, 2017, 6:29 p.m., David McLaughlin wrote: > > Do we have any idea how this performs at scale? I'd like to avoid repeating > > the MyBatis work, where we delayed the scale testing until after we had > > committed a bunch of patches. A fair concern! Until i can perform a more

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-04 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review192722 --- Just took a quick peek at this, a couple of drive by questions...

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-03 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review192666 --- Do we have any idea how this performs at scale? I'd like to avoid

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review192664 --- Ship it! Master (89338dd) is green with this patch.

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-03 Thread Bill Farner
> On Dec. 3, 2017, 5:22 p.m., Santhosh Kumar Shanmugham wrote: > > Can you list the details that will benefit us for pikcing HikariCP? Beyond what i wrote in the description, minimal dependencies (just slf4j) was the biggest. The benchmarks look favorable (and compare to the options i chose

Re: Review Request 64288: Add a SQL persistence implementation

2017-12-03 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/#review192662 --- Can you list the details that will benefit us for pikcing

Review Request 64288: Add a SQL persistence implementation

2017-12-03 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64288/ --- Review request for Aurora, David McLaughlin, Jordan Ly, and Stephan Erb.