Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-24 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review189160 --- Master (38476ab) is green with this patch.

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-24 Thread Bill Farner
> On Oct. 24, 2017, 9:53 a.m., Jordan Ly wrote: > > Overall LGTM! Tested upgrade/downgrade paths at scale and it seems to work > > well. > > Stephan Erb wrote: > Pure curiosity on my end: the benchmarks show pretty impressive speed > improvements. Are those also noticeable in practice for

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-24 Thread Stephan Erb
> On Okt. 24, 2017, 6:53 nachm., Jordan Ly wrote: > > Overall LGTM! Tested upgrade/downgrade paths at scale and it seems to work > > well. Pure curiosity on my end: the benchmarks show pretty impressive speed improvements. Are those also noticeable in practice for you? - Stephan

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-24 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review189062 --- Ship it! Overall LGTM! Tested upgrade/downgrade paths at scale

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-21 Thread Jordan Ly
> On Oct. 21, 2017, 4 p.m., Bill Farner wrote: > > FYI i'm in no rush, but awaiting an explicit ship it from Jordan. Want to > > make sure everyone is on board with this! Apologies for the delay everyone! Code-wise the patch looks good, but I would like to test it at scale for extra

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-21 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188898 --- Ship it! Ship It! - David McLaughlin On Oct. 18, 2017, 6:11

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-21 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188897 --- FYI i'm in no rush, but awaiting an explicit ship it from Jordan.

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-19 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188808 --- Master (a935389) is green with this patch.

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-19 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188767 --- This patch does not apply cleanly against master (b9ede2f), do

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188627 --- Master (c638877) is green with this patch.

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-18 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188544 --- Ship it! Thanks. You have answered all my questions and the

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-18 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188542 --- This patch does not apply cleanly against master (b7e83a0), do

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-18 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/ --- (Updated Oct. 18, 2017, 11:11 a.m.) Review request for Aurora, Jordan Ly and

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-18 Thread Bill Farner
> On Oct. 17, 2017, 11:21 a.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java > > Lines 79-103 (original), 72-96 (patched) > > > > > > Should these be

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-18 Thread Bill Farner
> On Oct. 16, 2017, 10:24 a.m., Jordan Ly wrote: > > Overall LGTM as well! I believe the logic for upgrading and maintaining > > forwards/backwards compatibility is sound. Ensuring my understanding, the > > upgrade path is: > > > > 1. A new version with the memory stores is released. > > 2.

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-18 Thread Bill Farner
> On Oct. 12, 2017, 2:53 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobUpdateStore.java > > Lines 190-196 (patched) > > > > > > You can combine these, right? > >

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-18 Thread Bill Farner
> On Oct. 12, 2017, 2:05 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java > > Lines 43-52 (patched) > > > > > > With multiple accesses of `hostAttributes` this

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-17 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188327 --- Mostly looks good to me. Some questions about concurrency when

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-16 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review188148 --- Overall LGTM as well! I believe the logic for upgrading and

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-12 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review187872 --- This LGTM! Super excited about this. I focused my rview effort

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-12 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review187861 --- I am finished with the second part of my review. Looks very solid

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-11 Thread Stephan Erb
> On Oct. 10, 2017, 11:23 p.m., Stephan Erb wrote: > > api/src/main/thrift/org/apache/aurora/gen/storage.thrift > > Line 149 (original) > > > > > > Can we remove that already? Could it be the case that we have some

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review187675 --- Master (519e3df) is green with this patch.

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review187612 --- This patch does not apply cleanly against master (519e3df), do

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Bill Farner
> On Oct. 10, 2017, 2:23 p.m., Stephan Erb wrote: > > RELEASE-NOTES.md > > Line 4 (original), 4 (patched) > > > > > > This is a major change. We should proabably call it out here. Done. > On Oct. 10, 2017, 2:23

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review187581 --- I have a first set of comments, but will have to complete my

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review187591 --- Master (4a1fba3) is green with this patch.

Re: Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/#review187574 --- Master (4a1fba3) is red with this patch.

Review Request 62869: Exclusively use Map-based in-memory stores for primary storage

2017-10-10 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62869/ --- Review request for Aurora, Jordan Ly and Stephan Erb. Repository: aurora