Re: Review Request 30774: Fetcher Cache

2015-04-13 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review79838 --- src/tests/fetcher_cache_tests.cpp

Re: Review Request 30774: Fetcher Cache

2015-04-13 Thread Bernd Mathiske
On March 18, 2015, 11:05 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 491 https://reviews.apache.org/r/30774/diff/37/?file=897704#file897704line491 Why not just mock _fetch and do a barrier on it by giving it a promise in test? Bernd Mathiske wrote:

Re: Review Request 30774: Fetcher Cache

2015-04-13 Thread Bernd Mathiske
On April 12, 2015, 11:11 p.m., Timothy Chen wrote: src/tests/fetcher_cache_tests.cpp, line 308 https://reviews.apache.org/r/30774/diff/42/?file=922829#file922829line308 Not sure why you picked an arbitrary number 5 here, why not let it be passed in? OK, I will add an explanation

Re: Review Request 30774: Fetcher Cache

2015-04-13 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated April 13, 2015, 5:45 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-24 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 24, 2015, 6:57 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-22 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 21, 2015, 11:54 p.m.) Review request for mesos, Adam B,

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review77019 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review77025 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
On March 18, 2015, 11:05 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 406 https://reviews.apache.org/r/30774/diff/37/?file=897704#file897704line406 Do we call fetch even if we don't have anything to fetch? I think it will be a good idea to have a fast

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 18, 2015, 11:43 p.m.) Review request for mesos, Adam B,

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 19, 2015, 10:48 p.m.) Review request for mesos, Adam B,

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
On March 19, 2015, 9:40 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 379 https://reviews.apache.org/r/30774/diff/38/?file=899705#file899705line379 benh, what do you think of Bernd's contentionBarrier injection? commonly we always just mock the callback

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
On Feb. 25, 2015, 10:56 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 528 https://reviews.apache.org/r/30774/diff/19/?file=875881#file875881line528 return size.error(); Bernd Mathiske wrote: That does not compile. Timothy Chen wrote: It doesn't?

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
On Feb. 25, 2015, 10:30 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 686 https://reviews.apache.org/r/30774/diff/19/?file=875881#file875881line686 Seems like lookupEntry is only used here, and it's always coupled with a reference call. How

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
On Feb. 24, 2015, 11:26 p.m., Timothy Chen wrote: docs/fetcher.md, line 68 https://reviews.apache.org/r/30774/diff/17/?file=872869#file872869line68 Not sure if putting the struct here is a good idea, as it's most likely going to be changed in the future. Bernd Mathiske wrote:

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
On March 18, 2015, 11:05 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 503 https://reviews.apache.org/r/30774/diff/37/?file=897704#file897704line503 Since this is only called in one place, how about put this in ___fetch, pass it the future and check if it

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Bernd Mathiske
On March 18, 2015, 11:48 p.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 726 https://reviews.apache.org/r/30774/diff/37/?file=897704#file897704line726 I'm not sure I understand, the error is never logged and in the end we simply return 0 if os::find returns a

Re: Review Request 30774: Fetcher Cache

2015-03-19 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review77057 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-03-18 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review76902 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-03-17 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 17, 2015, 4:52 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-17 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 17, 2015, 6:59 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-12 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review75754 --- include/mesos/fetcher/fetcher.proto

Re: Review Request 30774: Fetcher Cache

2015-03-11 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 11, 2015, 10:50 a.m.) Review request for mesos, Adam B,

Re: Review Request 30774: Fetcher Cache

2015-03-11 Thread Bernd Mathiske
On March 9, 2015, 8:37 a.m., Joerg Schad wrote: include/mesos/mesos.proto, line 208 https://reviews.apache.org/r/30774/diff/33/?file=888347#file888347line208 Could you add a comment (i.e. backlink to the documention) reminding developers to update docs/fetcher.md when the

Re: Review Request 30774: Fetcher Cache

2015-03-11 Thread Bernd Mathiske
On March 6, 2015, 2:15 p.m., Timothy Chen wrote: include/mesos/fetcher/fetcher.proto, line 58 https://reviews.apache.org/r/30774/diff/32/?file=887350#file887350line58 It's harder to make a optional field required, but it's much easier the other way around. If we

Re: Review Request 30774: Fetcher Cache

2015-03-11 Thread Bernd Mathiske
On March 2, 2015, 8:13 p.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 759 https://reviews.apache.org/r/30774/diff/25/?file=882228#file882228line759 If you're incrementing all the time just to count, why not just get the size from list? Bernd Mathiske

Re: Review Request 30774: Fetcher Cache

2015-03-11 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 11, 2015, 5:47 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-11 Thread Bernd Mathiske
On March 10, 2015, 8:35 a.m., Benjamin Hindman wrote: src/launcher/fetcher.cpp, line 321 https://reviews.apache.org/r/30774/diff/33/?file=888350#file888350line321 Can you comment the relationship between the FetcherInfo::Item and the FetcherInfo here? Is the FetcherInfo::Item

Re: Review Request 30774: Fetcher Cache

2015-03-09 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review75700 --- docs/fetcher.md https://reviews.apache.org/r/30774/#comment122920

Re: Review Request 30774: Fetcher Cache

2015-03-09 Thread Bernd Mathiske
On March 9, 2015, 8:37 a.m., Joerg Schad wrote: src/tests/fetcher_cache_tests.cpp, line 134 https://reviews.apache.org/r/30774/diff/33/?file=888361#file888361line134 Can't we simulate SERIALIZED_TASK externally (as discussed)? In this we would not have several modes... The whole

Re: Review Request 30774: Fetcher Cache

2015-03-07 Thread Bernd Mathiske
On March 6, 2015, 2:15 p.m., Timothy Chen wrote: src/launcher/fetcher.cpp, line 178 https://reviews.apache.org/r/30774/diff/32/?file=887354#file887354line178 You log the extraction command but in this case don't log the copy command. I think to be consistent, let's

Re: Review Request 30774: Fetcher Cache

2015-03-07 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 7, 2015, 7:21 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-06 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 6, 2015, 5:46 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-06 Thread Bernd Mathiske
On March 6, 2015, 2:15 p.m., Timothy Chen wrote: include/mesos/fetcher/fetcher.proto, line 58 https://reviews.apache.org/r/30774/diff/32/?file=887350#file887350line58 It's harder to make a optional field required, but it's much easier the other way around. If we

Re: Review Request 30774: Fetcher Cache

2015-03-05 Thread Bernd Mathiske
On March 4, 2015, 4:39 p.m., Jay Buffington wrote: Hey Bernd, I'm really looking forward to this feature. There's a lot here, so I was hoping you could help me understand by responding to some of these questions: Why do you need the cache table data structure? Just use the

Re: Review Request 30774: Fetcher Cache

2015-03-05 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 5, 2015, 3:15 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-04 Thread Jay Buffington
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review75266 --- Hey Bernd, I'm really looking forward to this feature. There's a

Re: Review Request 30774: Fetcher Cache

2015-03-04 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 4, 2015, 4:54 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-03 Thread Bernd Mathiske
On March 2, 2015, 8:13 p.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 759 https://reviews.apache.org/r/30774/diff/25/?file=882228#file882228line759 If you're incrementing all the time just to count, why not just get the size from list? I am not incrementing

Re: Review Request 30774: Fetcher Cache

2015-03-03 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 3, 2015, 5:01 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-03 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 3, 2015, 9:53 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-03 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 3, 2015, 11:40 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-03 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 3, 2015, 9:30 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-02 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74885 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-03-02 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 2, 2015, 10:27 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-01 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 1, 2015, 2:35 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74695 --- Bad patch! Reviews applied: [30606, 30609, 30774] Failed command:

Re: Review Request 30774: Fetcher Cache

2015-03-01 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 1, 2015, 7:42 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-03-01 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74703 --- Patch looks great! Reviews applied: [30606, 30609, 30774] All

Re: Review Request 30774: Fetcher Cache

2015-03-01 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated March 1, 2015, 12:27 p.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74446 --- Patch looks great! Reviews applied: [30606, 30609, 30774] All

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74278 --- Patch looks great! Reviews applied: [30606, 30609, 30774] All

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Bernd Mathiske
On Feb. 25, 2015, 2:20 p.m., Till Toenshoff wrote: This really is an impressive project Bernd, tipping my hat here! I did not get all the way through, had to ignore the tests in this review (to be continued!). For playing a bit with this and for testing the functionality, it

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74271 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Bernd Mathiske
On Feb. 25, 2015, 10:30 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 686 https://reviews.apache.org/r/30774/diff/19/?file=875881#file875881line686 Seems like lookupEntry is only used here, and it's always coupled with a reference call. How

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Bernd Mathiske
On Feb. 25, 2015, 10:56 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 528 https://reviews.apache.org/r/30774/diff/19/?file=875881#file875881line528 return size.error(); That does not compile. On Feb. 25, 2015, 10:56 a.m., Timothy Chen wrote:

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 26, 2015, 6:03 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Bernd Mathiske
On Feb. 25, 2015, 10:06 a.m., Timothy Chen wrote: docs/fetcher.md, line 165 https://reviews.apache.org/r/30774/diff/18/?file=875537#file875537line165 Doesn't cache items have TTLs? Then frameworks should never get into content that is stale for a long time? TTLs are not

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Bernd Mathiske
On Feb. 25, 2015, 10:25 a.m., Timothy Chen wrote: src/slave/containerizer/fetcher.cpp, line 103 https://reviews.apache.org/r/30774/diff/19/?file=875881#file875881line103 How about introducing a Cache namespace, so then we can just do Fetcher::Cache::recover,

Re: Review Request 30774: Fetcher Cache

2015-02-26 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 26, 2015, 7:58 p.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74040 --- docs/fetcher.md https://reviews.apache.org/r/30774/#comment120509

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Timothy Chen
On Feb. 25, 2015, 7:26 a.m., Timothy Chen wrote: docs/fetcher.md, line 16 https://reviews.apache.org/r/30774/diff/17/?file=872869#file872869line16 The framework provides a list of CommandInfo::URI values as part of the TaskInfo message that becomes the input to the Mesos fetcher.

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74069 --- Bad patch! Reviews applied: [30609, 30606, 30774] Failed command:

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74043 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74053 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74048 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review73462 --- This really is an impressive project Bernd, tipping my hat here! I

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Till Toenshoff
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74155 --- src/slave/containerizer/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Till Toenshoff
On Feb. 25, 2015, 10:20 p.m., Till Toenshoff wrote: src/slave/containerizer/fetcher.cpp, lines 459-460 https://reviews.apache.org/r/30774/diff/18/?file=875547#file875547line459 So a force removes from cache -- can you explain why this is needed? Ow I got it :) ... a force is a

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 25, 2015, 2:19 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review74010 --- Bad patch! Reviews applied: [30609, 30606, 30774] Failed command:

Re: Review Request 30774: Fetcher Cache

2015-02-25 Thread Bernd Mathiske
On Feb. 24, 2015, 11:26 p.m., Timothy Chen wrote: docs/fetcher.md, line 16 https://reviews.apache.org/r/30774/diff/17/?file=872869#file872869line16 The framework provides a list of CommandInfo::URI values as part of the TaskInfo message that becomes the input to the Mesos

Re: Review Request 30774: Fetcher Cache

2015-02-23 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 23, 2015, 3:33 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-23 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review73553 --- Bad patch! Reviews applied: [30609, 30606, 30774] Failed command:

Re: Review Request 30774: Fetcher Cache

2015-02-22 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 22, 2015, 10:09 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-22 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 22, 2015, 12:39 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-19 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 19, 2015, 7:14 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-18 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 18, 2015, 12:28 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-18 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 18, 2015, 12:07 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-18 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 18, 2015, 4:43 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-17 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review72822 --- Bad patch! Reviews applied: [30609, 30606] Failed command:

Re: Review Request 30774: Fetcher Cache

2015-02-17 Thread Bernd Mathiske
On Feb. 17, 2015, 3:02 a.m., Alexander Rukletsov wrote: src/tests/fetcher_cache_tests.cpp, line 117 https://reviews.apache.org/r/30774/diff/8/?file=866273#file866273line117 Don't you need to explicitly call `MesosTest::SetUp()`? Bernd Mathiske wrote: Nope. Revisited. I

Re: Review Request 30774: Fetcher Cache

2015-02-17 Thread Bernd Mathiske
On Feb. 17, 2015, 3:02 a.m., Alexander Rukletsov wrote: src/tests/fetcher_cache_tests.cpp, line 117 https://reviews.apache.org/r/30774/diff/8/?file=866273#file866273line117 Don't you need to explicitly call `MesosTest::SetUp()`? Nope. On Feb. 17, 2015, 3:02 a.m., Alexander

Re: Review Request 30774: Fetcher Cache

2015-02-17 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 17, 2015, 8:40 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-17 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review72694 --- src/tests/fetcher_cache_tests.cpp

Re: Review Request 30774: Fetcher Cache

2015-02-17 Thread Bernd Mathiske
On Feb. 8, 2015, 2:06 p.m., Benjamin Hindman wrote: src/tests/fetcher_cache_tests.cpp, line 438 https://reviews.apache.org/r/30774/diff/1/?file=857937#file857937line438 There is a ton of state, expectations, control flow requirements, etc, throughout these tests. This makes them

Re: Review Request 30774: Fetcher Cache

2015-02-15 Thread Bernd Mathiske
On Feb. 8, 2015, 2:06 p.m., Benjamin Hindman wrote: src/tests/fetcher_cache_tests.cpp, lines 332-334 https://reviews.apache.org/r/30774/diff/1/?file=857937#file857937line332 We've got a concurrent queue in libprocess called 'Queue'. See scheduler_tests.cpp for how it's used. I'd

Re: Review Request 30774: Fetcher Cache

2015-02-15 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 15, 2015, 2:29 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-15 Thread Bernd Mathiske
On Feb. 8, 2015, 2:06 p.m., Benjamin Hindman wrote: src/tests/fetcher_cache_tests.cpp, lines 132-135 https://reviews.apache.org/r/30774/diff/1/?file=857937#file857937line132 There is only checkpointing mode now, so don't bother trying to test both cases. Bernd Mathiske wrote:

Re: Review Request 30774: Fetcher Cache

2015-02-15 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 15, 2015, 7:28 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-14 Thread Bernd Mathiske
On Feb. 10, 2015, 1:26 a.m., Adam B wrote: Please update the description with a descriptive commit message. Also explain any testing you did for HDFS fetching (since it's half-broken without the HADOOP_HOME fix). Done. The description of the HDFS testing is under 30614. - Bernd

Re: Review Request 30774: Fetcher Cache

2015-02-14 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 14, 2015, 3 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-14 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 14, 2015, 1:41 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-10 Thread Bernd Mathiske
On Feb. 8, 2015, 2:06 p.m., Benjamin Hindman wrote: src/launcher/fetcher.cpp, line 388 https://reviews.apache.org/r/30774/diff/1/?file=857926#file857926line388 Kill '} else {'. That would be wrong. Then the CHECK would fire every time. On Feb. 8, 2015, 2:06 p.m., Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-10 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 10, 2015, 1:14 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache Working Copy for Ben and Bernd

2015-02-08 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 8, 2015, 8:47 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 30774: Fetcher Cache

2015-02-08 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/#review71575 --- src/launcher/fetcher.cpp

Re: Review Request 30774: Fetcher Cache

2015-02-08 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated Feb. 8, 2015, 11:34 a.m.) Review request for mesos, Adam B, Benjamin