Review Request 30969: Added getMethodID to determine when a method is missing.

://reviews.apache.org/r/30969/diff/ Testing --- This was tested through my API changes. Thanks, Ben Mahler

Review Request 30974: Updated test expectations for the statusUpdate API change.

Diff: https://reviews.apache.org/r/30974/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30978: Updated documentation for explicit acknowledgement API change.

e6247b23de3aebbe911a2f72839710c7477008fe docs/upgrades.md 168e761b7242dc421ff6c7748e2c735de07ab87c Diff: https://reviews.apache.org/r/30978/diff/ Testing --- N/A Thanks, Ben Mahler

Review Request 30976: Updated example frameworks for explicit acknowledgements.

Diff: https://reviews.apache.org/r/30976/diff/ Testing --- make check with and without MESOS_EXPLICIT_ACKNOWLEDGEMENTS Thanks, Ben Mahler

Review Request 30973: Updated Python bindings for explicit acknowledgements.

501c5747ab4f269ebdfd677ccf15aed38cc6e92b src/python/native/src/mesos/native/proxy_scheduler.cpp a6e1d24ad3191f853fd563a613e41d6986afeab6 Diff: https://reviews.apache.org/r/30973/diff/ Testing --- Tested in subsequent patches. Thanks, Ben Mahler

Review Request 30971: Introduced explicit status update acknowledgements on the driver.

ea7e447e522f8fa335ee5dbdc6d65d4018042905 Diff: https://reviews.apache.org/r/30971/diff/ Testing --- Added tests in subsequent reviews. Thanks, Ben Mahler

Re: Review Request 30906: Added a metric for launcher's container destruction failure.

> On Feb. 12, 2015, 10:53 p.m., Alexander Rukletsov wrote: > > src/slave/containerizer/linux_launcher.cpp, line 418 > > > > > > Maybe it makes sense to mimic the directory structure in naming for > > simplicty and con

Re: Review Request 30906: Added a metric for launcher's container destruction failure.

> On Feb. 12, 2015, 5:41 p.m., Ben Mahler wrote: > > src/slave/containerizer/mesos/containerizer.hpp, line 280 > > <https://reviews.apache.org/r/30906/diff/1/?file=861366#file861366line280> > > > > Should this just be "mesos_containerizer/launche

Re: Review Request 30906: Added a metric for launcher's container destruction failure.

tps://reviews.apache.org/r/30906/#comment118193> Should this just be "mesos_containerizer/launcher_destroy_errors"? Looking at our port_mapping/ metrics, and other subcomponent metrics, that seems like a more consistent name? - Ben Mahler On Feb. 12, 2015, 1:34 a.m.,

Re: Review Request 30827: Removed deprecated slave_id from ReregisterSlaveMessage.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30827/#review72078 --- Ship it! Ship It! - Ben Mahler On Feb. 11, 2015, 10:19 a.m

Re: Review Request 30886: Remove more non-pod statics from clock

tps://reviews.apache.org/r/30886/#comment117939> Remove this? - Ben Mahler On Feb. 11, 2015, 7:55 p.m., Dominic Hamon wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Review Request 30884: Removed non-POD static function object in clock.

page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14316710). Diffs - 3rdparty/libprocess/src/clock.cpp fcc1eb0b921b3d9b9c033a17e5f6dcecf0a30f08 Diff: https://reviews.apache.org/r/30884/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 30850: Added validation for checkpointed resources during slave recovery.

/#comment117814> s/is/are/ src/tests/mesos.cpp <https://reviews.apache.org/r/30850/#comment117817> Can you pull out the removal of 'process::' and just commit it as a patch on master? - Ben Mahler On Feb.

Re: Review Request 30812: Updated the allocator in Master::addSlave using slave->totalResources.

the checkpointed resources are offered? (in a follow up patch) - Ben Mahler On Feb. 10, 2015, 1:35 a.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 30827: Removed deprecated slave_id from ReregisterSlaveMessage.

> This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30827/ > ----------- > > (Updated Feb. 10, 2015, 3:04 p.m.) > > > Review request for mesos, A

Re: Review Request 30609: Added os::lstatsize().

avoid needing to introduce so many special case functions. - Ben Mahler On Feb. 10, 2015, 11:19 a.m., Bernd Mathiske wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 30580: Updated the graceful shutdown documentation and naming.

tive grace period (taken that we have a hierarchy of graceful > > shutdowns which need this period + delta to allow enough time for each > > step)? > > Ben Mahler wrote: > Hm, this mentions that this the command executor timeout and the > driver/slave use slightly

Re: Review Request 27531: Update Master metrics to match task source and reason scheme.

> On Feb. 6, 2015, 6:56 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 3601 > > > > > > Looks like you missed updating metrics here? > > > > Now that I think about it, it's worthwhile to consolidate

Re: Review Request 30784: Fixed flaky test MasterAllocatorTest/0.OutOfOrderDispatch.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30784/#review71688 --- Ship it! Ship It! - Ben Mahler On Feb. 9, 2015, 8:08 p.m

Re: Review Request 30792: Additonal use case for "auto" in C++ style guide: shared pointer creation.

os-c++-style-guide.md <https://reviews.apache.org/r/30792/#comment117546> How about s/nameList/names/ to discourage including type information redundantly in variable names? - Ben Mahler On Feb. 9, 2015, 10:30 a.m., Bernd Mathiske wrote: > > ---

Re: Review Request 30784: Fixed flaky test MasterAllocatorTest/0.OutOfOrderDispatch.

the stop the driver down below? Also, use DoDefault() for these. - Ben Mahler On Feb. 9, 2015, 4:02 a.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-mail. To reply, vi

Re: Review Request 30694: Fixed bugs in CREATE/DESTROY operation handlers and added tests.

694/#comment117246> It's not obvious that there's enough disk for the 64MB and 128MB volumes below, why not just set `--resources` here? - Ben Mahler On Feb. 6, 2015, 7:47 p.m., Jie Yu wrote: > > --- > This is

Re: Review Request 30580: Updated the graceful shutdown documentation and naming.

omatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30580/#review71493 ----------- On Feb. 4, 2015, 2:14 a.m., Ben Mahler wrote: > > --- > This is an auto

Re: Review Request 30694: Fixed bugs in CREATE/DESTROY operation handlers and added tests.

r/30694/#comment117264> s/bytes/size/ ? src/tests/persistent_volume_tests.cpp <https://reviews.apache.org/r/30694/#comment117265> Maybe you want a note that currently we send 1 message per operation? Technically, that's a bit of an implementation detail, we could send 1 p

Re: Review Request 30694: Fixed bugs in CREATE/DESTROY operation handlers and added tests.

g/r/30694/#comment117271> Should we be failing when there's a checkpointed resource that we've ignored here? That is, do you want an else block with a LOG(FATAL) to ensure we don't miss checkpointed resources here? - Ben Mahler On Feb. 6, 2015, 11:

Re: Review Request 30728: Cleaned up extra declaration and whitespace.

ply, visit: > https://reviews.apache.org/r/30728/ > --- > > (Updated Feb. 6, 2015, 2:59 p.m.) > > > Review request for mesos, Ben Mahler, Kapil Arya, and Timothy Chen. > > > Repository: mesos > > > Des

Re: Review Request 30728: Cleaned up extra declaration and whitespace.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30728/#review71458 --- Ship it! Ship It! - Ben Mahler On Feb. 6, 2015, 2:59 p.m

Re: Review Request 30694: Fixed bugs in CREATE/DESTROY operation handlers and added tests.

eResources.apply(operation.create()); CHECK_SOME(resources); slaveResources = resources.get(); slave->checkpointedResources = slaveResources.filter(checkpointFilter); ``` This approach should work for the DESTROY case as well, thoughts? - Ben Mahler On Feb. 5, 2015

Re: Review Request 30601: Updated slave to use Executor/Task grace period, with a maximum.

e.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 Diff: https://reviews.apache.org/r/30601/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 30601: Updated slave to use Executor/Task grace period, with a maximum.

erizer.cpp 421bb868b353e644578fa27f04bdd636bfc89134 src/slave/slave.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 Diff: https://reviews.apache.org/r/30601/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 30601: Updated slave to use Executor/Task grace period, with a maximum.

o be consistent. Thanks! - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30601/#review71119 --- On Feb. 4, 2

Re: Review Request 30601: Updated slave to use Executor/Task grace period, with a maximum.

y, visit: https://reviews.apache.org/r/30601/#review71062 ----------- On Feb. 4, 2015, 10:46 p.m., Ben Mahler wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 30601: Updated slave to use Executor/Task grace period, with a maximum.

ECUTOR_SHUTDOWN_GRACE_PERIOD_MAXIMUM and it actually works as lower limit. Good catch! - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30601/#review71062 ---------

Re: Review Request 30583: Fixed MESOS_RECOVERY_TIMEOUT to be based on the flag value.

e set the default grace > > period in getExecutorInfo() (and thereby _guaranteeing_ that the executor's > > command info always have a grace period set?) We do this for the default > > container. > > > > Know we talked about this before, but forgot whether we go

Re: Review Request 30580: Updated the graceful shutdown documentation and naming.

o you have a concrete suggestion of how to rephrase this? - Ben ----------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30580/#review71060

Re: Review Request 30583: Fixed MESOS_RECOVERY_TIMEOUT to be based on the flag value.

- On Feb. 4, 2015, 10:43 p.m., Ben Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30583/ >

Re: Review Request 30583: Fixed MESOS_RECOVERY_TIMEOUT to be based on the flag value.

-- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30583/#review71067 --- On Feb. 4, 2015, 10:43 p.m., Ben Mahler wrote: > > --

Re: Review Request 28809: Started to maintain and checkpoint persisted resource in slave.

slave/slave.cpp <https://reviews.apache.org/r/28809/#comment116955> It's too bad the `errors` are just unsigned ints and not a collection of `Option`s that we can print here. - Ben Mahler On Feb. 4, 2015, 7:13 p.m., Jie Yu wrote: > > --

Re: Review Request 30584: Added metrics for slave shutdowns.

src/master/metrics.cpp <https://reviews.apache.org/r/30584/#comment116696> Indentation? - Ben Mahler On Feb. 5, 2015, 12:13 a.m., Vinod Kone wrote: > > --- > This is an automatically generated e-mail. To repl

Re: Review Request 30514: Rate limited the removal of slaves failing health checks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30514/#review71098 --- Ship it! Ship It! - Ben Mahler On Feb. 5, 2015, 12:13 a.m

Re: Review Request 30644: Added implementation for DESTROY operation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30644/#review71085 --- Ship it! Ship It! - Ben Mahler On Feb. 4, 2015, 11:53 p.m., Jie

Re: Review Request 30642: Added validation for DESTROY operation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30642/#review71083 --- Ship it! Ship It! - Ben Mahler On Feb. 4, 2015, 11:52 p.m., Jie

Re: Review Request 30514: Rate limited the removal of slaves failing health checks.

wo l's and sometimes using one l :) src/tests/slave_tests.cpp <https://reviews.apache.org/r/30514/#comment116605> Looks good. Do you think we want to inject a rate limiter at some point? - Ben Mahler On Feb. 4, 2015, 10:43 p.m., Vinod Kone wrote: > >

Re: Review Request 30601: Updated slave to use Executor/Task grace period, with a maximum.

src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 Diff: https://reviews.apache.org/r/30601/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 30601: Updated slave to use Executor/Task grace period, with a maximum.

set explicitly. Thanks! - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30601/#review70960 --- On Feb. 4,

Re: Review Request 30583: Fixed MESOS_RECOVERY_TIMEOUT to be based on the flag value.

here is a non-empty value and we cannot parse it, which is indicative of a more serious error. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30583/#review70953 -----

Re: Review Request 30583: Fixed MESOS_RECOVERY_TIMEOUT to be based on the flag value.

62a2c6 Diff: https://reviews.apache.org/r/30583/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 30580: Updated the graceful shutdown documentation and naming.

n > > flags loading functionality, though. I've added a TODO to have a EXECUTOR_SHUTDOWN_GRACE_PERIOD_MAXIMUM as well, in: https://reviews.apache.org/r/30601/ - Ben --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 30597: Fixed the executor driver and command executor to read the graceful shutdown time.

://reviews.apache.org/r/30601/ Diffs - src/exec/exec.cpp aada24664dba9060a92230e25689c89852585443 src/launcher/executor.cpp 1cf28f168cac6e8c7e98686a35509c2b4e052504 Diff: https://reviews.apache.org/r/30597/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 29742: Added useful utility functions to determine types of resources.

operations, or? - Ben Mahler On Jan. 29, 2015, 5:27 a.m., Michael Park wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 30386: Added support for CREATE operation in master.

))? The caller is expected not to call this with anything that might get dropped in here, right? - Ben Mahler On Feb. 4, 2015, 1:09 a.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visi

Re: Review Request 30635: Fixed MESOS-2319 by creating the temporary file under the same base directory.

NOTE or TODO about how to address the issue of leaving dangling files when we failover at the wrong time? - Ben Mahler On Feb. 4, 2015, 7:38 p.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 30514: Rate limited the removal of slaves failing health checks.

> On Feb. 4, 2015, 8:01 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 197-246 > > <https://reviews.apache.org/r/30514/diff/5/?file=847032#file847032line197> > > > > It looks like we only log when we're shutting down the sl

Re: Review Request 30514: Rate limited the removal of slaves failing health checks.

ed out"); } else if (future.isDiscarded()) { // Shutdown was canceled. LOG(INFO) << "Canceling shutdown of slave " << slaveId << " since a pong is received!"; ++metrics->slave_shutdowns_can

Review Request 30580: Updated the graceful shutdown documentation and naming.

e7e2af63da785644f3f7e6e23607c02be962a2c6 Diff: https://reviews.apache.org/r/30580/diff/ Testing (updated) --- make check Thanks, Ben Mahler

Review Request 30597: Fixed the executor driver and command executor to read the graceful shutdown time.

- src/exec/exec.cpp aada24664dba9060a92230e25689c89852585443 src/launcher/executor.cpp 1cf28f168cac6e8c7e98686a35509c2b4e052504 Diff: https://reviews.apache.org/r/30597/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30595: Fixed the slave to set up the executor shutdown timeout based on the ExecutorInfo.

ee: https://reviews.apache.org/r/30601/ Diffs - src/slave/slave.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b Diff: https://reviews.apache.org/r/30595/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30601: Updated slave to use Executor/Task grace period, with a maximum.

rizer/containerizer.cpp 421bb868b353e644578fa27f04bdd636bfc89134 src/slave/slave.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 Diff: https://reviews.apache.org/r/30601/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30583: Fixed MESOS_RECOVERY_TIMEOUT to be based on the flag value.

/mesos.cpp 5ed4df530cf1bf11eec3b29542641822e0f702b2 src/tests/slave_recovery_tests.cpp 7e2e63d4b8a1cd7c191374bc37073d83ae413e03 src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 Diff: https://reviews.apache.org/r/30583/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30596: Updated the slave to set the graceful shutdown for the command executor.

a8b262174ab5c9a524db8318d3d1438cd75a702b Diff: https://reviews.apache.org/r/30596/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30579: Added a missing newline in the command executor output.

(updated) --- make check Thanks, Ben Mahler

Re: Review Request 29329: Add executor for docker containerizer

> On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote: > > How are we going to manage the duplication across the command executor and > > the docker executor? > > Timothy Chen wrote: > I think I'm going to leave them seperate as they most likely will grow > i

Re: Review Request 30511: Moved framework related rate limiters into Master::Frameworks.

ls a future when capacity is exceeded, if possible. I believe there are some subtleties there IIRC). src/master/master.cpp <https://reviews.apache.org/r/30511/#comment116042> Hm.. mind indenting this to show that it's a continuation of the < operation for the last line? - Ben

Re: Review Request 30536: Renamed persisted resources to checkpointed resources.

5> s/currelnty/currently/ here and above. But, I think you'll want to just change this to use "e.g." like your other comments, otherwise this comment is easily going to become outdated when we add other checkpointed resources. - Ben Mahler On Feb. 3, 20

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

ws.apache.org/r/30459/ > ------- > > (Updated Jan. 31, 2015, 2:09 a.m.) > > > Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone. > > > Bugs: MESOS-2305 > https://issues.apache.org/jira/brow

Re: Review Request 30459: Refactored task/offer/resource valiation in master.

he existing semantics and it's unnecessary work. Could we use lambda::bind per our chat to avoid this? - Ben Mahler On Jan. 30, 2015, 10:27 p.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply,

Review Request 30463: Increased the default AWAIT timeout.

t.hpp 10f991d4e6a6ec8479f342ee812faa3f5c3870bb Diff: https://reviews.apache.org/r/30463/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30439: Increased the default AWAIT timeout.

org/r/30439/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30432: Fixed the flaky MasterAuthorizationTest.FrameworkRemovedBeforeReregistration test.

fe9b5c3 Diff: https://reviews.apache.org/r/30432/diff/ Testing --- 500 iterations of this test Thanks, Ben Mahler

Re: Review Request 30430: Fixed a bug in Resources::contains.

<https://reviews.apache.org/r/30430/#comment115377> Maybe s/Subset/Contains/ and we can add a normal (non-duplicate) contains test in here? - Ben Mahler On Jan. 29, 2015, 11:08 p.m., Jie Yu wrote: > > --- > This is a

Re: Review Request 30386: Added support for CREATE operation in master.

> On Jan. 29, 2015, 6:49 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 2929-2931 > > <https://reviews.apache.org/r/30386/diff/1/?file=839423#file839423line2929> > > > > What is this TODO? How could one create a volume that's already

Review Request 30423: Fixed the flaky FaultToleranceTest.eSchedulerFailoverFrameworkMessage test.

100 repetitions of this test Thanks, Ben Mahler

Re: Review Request 28781: Maintained persisted resources in master memory.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28781/#review70254 --- Ship it! Ship It! - Ben Mahler On Jan. 29, 2015, 7:16 p.m., Jie

Re: Review Request 30402: Increased the timeout for the flaky SlaveTest.CommandExecutorGracefulShutdown test.

re > > readers? Thanks! - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30402/#review70243 ------- On Jan

Re: Review Request 29927: Implemented MasterAllocatorTest::StopAllocator() method.

> On Jan. 27, 2015, 7:55 p.m., Ben Mahler wrote: > > src/tests/master_allocator_tests.cpp, lines 80-81 > > <https://reviews.apache.org/r/29927/diff/3/?file=836337#file836337line80> > > > > Can we be very explicit here about why we need this, and what is &g

Re: Review Request 30386: Added support for CREATE operation in master.

; What is this TODO? How could one create a volume that's already reserved? - Ben Mahler On Jan. 29, 2015, 12:12 a.m., Jie Yu wrote: > > --- > This is an a

Re: Review Request 30402: Increased the timeout for the flaky SlaveTest.CommandExecutorGracefulShutdown test.

reply, visit: https://reviews.apache.org/r/30402/#review70183 ------- On Jan. 29, 2015, 4:35 a.m., Ben Mahler wrote: > > --- > This is an automatically

Review Request 30402: Increased the timeout for the flaky SlaveTest.CommandExecutorGracefulShutdown test.

;. Diffs - src/launcher/executor.cpp f00b6fcc2976b210c6213e52662f18f0d0342671 src/tests/slave_tests.cpp aff9e255bac596a02c3d31b7c11dd5389634be20 Diff: https://reviews.apache.org/r/30402/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 28781: Maintained persisted resources in master memory.

(vector) constructor help you here? Or are you intentionally passing around a vector? src/master/master.hpp <https://reviews.apache.org/r/28781/#comment115142> Any reason why you're not doing this in the initializer list via the Resources(vector) constructor? - Ben Mahler On Ja

Re: Review Request 29927: Implemented MasterAllocatorTest::StopAllocator() method.

> On Jan. 27, 2015, 7:55 p.m., Ben Mahler wrote: > > src/tests/master_allocator_tests.cpp, lines 80-81 > > <https://reviews.apache.org/r/29927/diff/3/?file=836337#file836337line80> > > > > Can we be very explicit here about why we need this, and what is &g

Re: Review Request 29991: Removed unnecessary allocator expectations in the tests.

tomatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29991/#review68601 ------- On Jan. 16, 2015, 11:12 p.m., Ben Mahler wrote: > > --

Re: Review Request 29991: Removed unnecessary allocator expectations in the tests.

en --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29991/#review68776 ------- On Jan. 16, 2015, 11:12 p.m., Ben Mahler wrote: > > -

Re: Review Request 29990: Updated TestAllocatorProcess to avoid the test warnings.

of this file? Yep, done. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29990/#review68769 --- On Jan. 16, 2015, 11:12 p.m., Ben Mahler wrote: > > -

Re: Review Request 28781: Maintained persisted resources in master memory.

patch? Just having this patch add the persistent resources should be pretty simple. - Ben Mahler On Jan. 28, 2015, 1:34 a.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://review

Re: Review Request 30298: Refactored resources validations into separate validators.

t; "Invalid DiskInfo: expecting 'volume' to be set" ``` src/master/master.cpp <https://reviews.apache.org/r/30298/#comment115087> Maybe: "Invalid Persistence ID: 'XXX' is not unique" - Ben Mahler On J

Re: Review Request 30192: Updated RateLimiter to allow discard semantics.

tps://reviews.apache.org/r/30192/#comment115080> Do the comments here add value? 3rdparty/libprocess/src/tests/limiter_tests.cpp <https://reviews.apache.org/r/30192/#comment115081> Ditto here. - Ben Mahler On Jan. 28, 2015, 1:36 a.m., Vino

Re: Review Request 30348: Clear all callbacks when a future is completed.

tps://reviews.apache.org/r/30348/#comment115078> Any reason to have these comments? We added internal::run to make it obvious that we're running the callbacks, these comments seem to not add any insight :) Ditto above. - Ben Mahler On Jan. 28, 2015, 10 p.m., Vinod

Re: Review Request 30283: Separated offer operations in Master::_accept

g/r/30283/#comment115056> Looks like there's no space at the end. We should also probably print the string in these cases where we know the type. ``` "Unsupported offer operation: '" << Offer::Operation::Type_Name(operation.type()) << "'&q

Review Request 30353: Fixed a race in Cluster shutdown when destroying containers.

This fixes the flaky SlaveRecoveryTest.ReconcileKillTask test in MESOS-2283. Diffs - src/tests/cluster.hpp 74cedb324949143fd1949082d8a4db596a32d95c Diff: https://reviews.apache.org/r/30353/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 30352: Added an AWAIT macro for testing that futures transition out of pending.

t.hpp b77e2fc0eba18e2099bcb759a500abd271548428 Diff: https://reviews.apache.org/r/30352/diff/ Testing --- make check (used this in the subsequent patch) Thanks, Ben Mahler

Re: Review Request 30347: Added additional details to developers guide.

hanks Dave! - Ben Mahler On Jan. 28, 2015, 1:32 a.m., David Robinson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

Re: Review Request 30328: Added external_log_file option.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30328/#review69932 --- Ship it! Ship It! - Ben Mahler On Jan. 27, 2015, 9:57 p.m

Re: Review Request 29927: Implemented MasterAllocatorTest::StopAllocator() method.

g/r/29927/#comment114688> Can we be very explicit here about why we need this, and what is causing the flakiness? How does this relate to my patches to silence the mock warnings? - Ben Mahler On Jan. 27, 2015, 3:23 p.m., Alexander Rukletsov

Re: Review Request 30192: Updated RateLimiter to allow discard semantics.

They are all pending, it's just that some of them have a discard request :) Mind updating the comment? - Ben Mahler On Jan. 27, 2015, 12:18 a.m., Vinod Kone wrote: > > --- > This is an automatically generated e-m

Re: Review Request 29071: added webui_log option

ave/slave.cpp <https://reviews.apache.org/r/29071/#comment114438> Ditto here. src/webui/master/static/js/controllers.js <https://reviews.apache.org/r/29071/#comment114442> If `external_log_file` ends up overriding `log_dir` instead of being mutually exclusive with it

Re: Review Request 30237: Introduced RunStateBase that is derived to create RunState.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30237/#review69654 --- Why is a base class needed here? - Ben Mahler On Jan. 24, 2015

Re: Review Request 30131: Used persistent volumes consistently in the code base.

this. include/mesos/resources.hpp <https://reviews.apache.org/r/30131/#comment114386> Not yours, but should this be an Option? src/tests/resources_tests.cpp <https://reviews.apache.org/r/30131/#comment114391> volume1 and volume2? Like you did below? - Ben Mahler On Jan.

Re: Review Request 30130: Removed Resources::Transformation in favor of using Offer::Operation.

g/r/30130/#comment114380> Can we add an example in this comment? // ... additional metadata (e.g. volumes). src/tests/resources_tests.cpp <https://reviews.apache.org/r/30130/#comment114381> s/Transformation/Operation/ ? - Ben Mahler On Jan. 23, 2015, 5:16 p.m.,

Re: Review Request 30131: Used persistent volumes consistently in the code base.

views.apache.org/r/30131/#comment113707> Can you consolidate this one with what you're adding? - Ben Mahler On Jan. 21, 2015, 6:12 p.m., Jie Yu wrote: > > --- > This is an automatically generated e-mai

Re: Review Request 30130: Killed Resources::Transformation in favor of using Offer::Operation.

now that we avoid using "transformation"? src/master/hierarchical_allocator_process.hpp <https://reviews.apache.org/r/30130/#comment113686> What happened here? Mind leaving as is to make the diff smaller? - Ben Mahler On Jan. 21, 2015, 6:11 p.m., Jie Yu wrote

<    1   2   3   4   5   6   7   8   9   10   >