Re: Review Request 39845: Added REASON_RESOURCE_OVERSUBSCRIBED to mesos proto.

org/r/39845/#comment176043> To Vinod's point; can we reuse the preempted status? 'Resource oversubscribed' seems a bit counter intuitive as a error message, as that is what you tried to do in the first place :) - Niklas Nielsen On Jan. 19, 2016, 2:39 a.m., Guangya Liu

Re: Review Request 39848: Validate revocable resources

_total < task.resources().revocable()) { ... } Also, any reason this need to happen in this continuation of runTask() and not the earlier one? - Niklas Nielsen On Jan. 19, 2016, 2:39 a.m., Guangya Liu wrote: > > --- > This i

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

- > > (Updated Dec. 10, 2015, 9:03 a.m.) > > > Review request for mesos, Niklas Nielsen and Vinod Kone. > > > Bugs: MESOS-4076 > https://issues.apache.org/jira/browse/MESOS-4076 > > > Repository: mesos > > > Description >

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

> On Dec. 9, 2015, 10:32 a.m., Niklas Nielsen wrote: > > src/slave/qos_controllers/load.hpp, lines 39-45 > > <https://reviews.apache.org/r/40617/diff/5/?file=1155926#file1155926line39> > > > > Awesome comment! Let's make it a doxygen style one. > > Bar

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

-- > > (Updated Dec. 10, 2015, 9:03 a.m.) > > > Review request for mesos, Niklas Nielsen and Vinod Kone. > > > Bugs: MESOS-4076 > https://issues.apache.org/jira/browse/MESOS-4076 > > > Repository: mesos > > > Description > --- > >

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

oller when changing the values of the stub load, or how do we guarantee that? src/tests/oversubscription_tests.cpp (lines 1142 - 1149) <https://reviews.apache.org/r/40617/#comment169156> Wonder if we can reduce some of this boiler plate; have you taken a look at `create

Re: Review Request 41042: Added description of the LoadQoSController in the oversubscription.md

ttps://reviews.apache.org/r/41042/#comment168634> Same here for 15min - Niklas Nielsen On Dec. 7, 2015, 9:18 a.m., Bartek Plotka wrote: > > --- > This is an automatically generated e-mail. To reply

Re: Review Request 40056: Make hook execution order deterministic.

> On Dec. 4, 2015, 1:15 p.m., Niklas Nielsen wrote: > > Hi Haosdent! > > > > I apologize the tardy reply. The patch looks good but needs rebasing. > > Also, have you thought of a way to test this? > > > > With a test (maybe by just ensuring the exi

Re: Review Request 37540: Add perf event API

> On Dec. 2, 2015, 10:13 a.m., Niklas Nielsen wrote: > > Ping - Cong, is there anything you need to close the last issues? :) > > Cong Wang wrote: > I was blocked at the second to the last issue above and switched to > something else, now I can think more about it. T

Re: Review Request 39781: Update ModuleTest to not assume dynamic dlopen search.

ly, visit: > https://reviews.apache.org/r/39781/ > --- > > (Updated Nov. 26, 2015, 6:04 a.m.) > > > Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and > Till Toenshoff. > > > Bugs: MES

Re: Review Request 40056: Make hook execution order deterministic.

but needs rebasing. Also, have you thought of a way to test this? With a test (maybe by just ensuring the existing ordering of the test modules are indeed run in order), we should be able to land this. Kapil, any concerns on this? - Niklas Nielsen On Nov. 8, 2015, 4:58 a.m., haosdent huang wrote

Re: Review Request 39782: Add a comment for os::libraries::setPaths.

either the Linux nor the OS X // linkers dynamically update their search path after a process is launched. ``` - Niklas Nielsen On Nov. 26, 2015, 6:06 a.m., James Peach wrote: > > --- > This is an automatically generated e

Re: Review Request 40617: Added Load QoS Controller for simple eviction when system load is above configured threshold.

rg/r/40617/#comment168489> Doesn't this need to be indented? src/tests/oversubscription_tests.cpp (line 1172) <https://reviews.apache.org/r/40617/#comment168491> We should probably add some documentation on where these numbers come from - Niklas Nielsen On Nov. 26, 2015, 4:05

Re: Review Request 37540: Add perf event API

? :) - Niklas Nielsen On Sept. 29, 2015, 5:12 p.m., Cong Wang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 38627: Adds an overload of ModuleManager::create() allowing overriding parameters programatically

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38627/#review103837 --- LGTM - Kapil, any thoughts? - Niklas Nielsen On Oct. 21, 2015

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

g/r/39531/#comment161759> Can we inline JSON::Protobuf() instead? src/examples/test_hook_module.cpp (line 261) <https://reviews.apache.org/r/39531/#comment161760> 0.27.0? - Niklas Nielsen On Oct. 22, 2015, 4:30 p.m., Connor Doyle wrote: > > --

Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

eviews.apache.org/r/39388/#comment161156> Can we make this assumption (do we have other places where we assume localhost is always 127.0.0.1)? - Niklas Nielsen On Oct. 17, 2015, 4:18 p.m., Michael Park wrote: > > --- > This

Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

g/r/39388/#comment160790> Maybe be a bit more explicit: "... in cases where LIBPROCESS_IP has been set explicitly in host environment. Launching executors within docker containers in cases where libprocess cannot resolve the IP to bind to, will otherwise fail." - Niklas Niel

Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

> On Oct. 16, 2015, 11:18 a.m., Niklas Nielsen wrote: > > Also, let's get a test wired up to verify that this works :) - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.or

Re: Review Request 39386: Fix uncorrect launcher dir in docker executor.

386/#comment160853> What happens when you remove this fallback, taken a user don't provide the launcher dir? - Niklas Nielsen On Oct. 15, 2015, 10:17 p.m., haosdent huang wrote: > > --- > This is an automatically generated e

Re: Review Request 38570: Change documentation image links to absolute paths.

e trailing slashes from all > documentation links. (Maybe add redirects from trailing slash pages to > non-trailing slash pages.) > > > What do you think? > > Niklas Nielsen wrote: > BenM: ping ^^ > > Ben Mahler wrote: > Removing the trailing sla

Re: Review Request 38627: Adds an overload of ModuleManager::create() allowing overriding parameters programatically

--- > > (Updated Oct. 2, 2015, 1:11 p.m.) > > > Review request for mesos, Adam B, Bernd Mathiske, Niklas Nielsen, and Till > Toenshoff. > > > Bugs: MESOS-3232 > https://issues.apache.org/jira/browse/MESOS-3232 > > > Repository: mesos > > &g

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

utomatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38564/ > --- > > (Updated Oct. 12, 2015, 6:39 p.m.) > > > Review request for mesos, Connor Doyle and Niklas Nielsen. > > > Bug

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

tps://reviews.apache.org/r/39152/#comment160178> Shouldn't we only set this if it is not present? - Niklas Nielsen On Oct. 9, 2015, 1:34 p.m., Greg Mann wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 38570: Change documentation image links to absolute paths.

> On Oct. 6, 2015, 2:29 p.m., Ben Mahler wrote: > > docs/external-containerizer.md, line 92 > > > > > > How will this work when we add multiple versions of documents to the > > website? Now this hardcodes "latest"

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

Let's rename this to 'slaveAttributesDecorator': here and in rest of patch src/tests/hook_tests.cpp (line 659) <https://reviews.apache.org/r/38564/#comment159977> Should we rename this test as you are testing both decorators here? If not, maybe create a seperate test for it. - Niklas Nielsen On

Re: Review Request 39205: Deprecate resource_monitoring_interval flag

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39205/#review102361 --- Ship it! Ship It! - Niklas Nielsen On Oct. 11, 2015, 8:32 p.m

Re: Review Request 38279: Add a new callback enabling custom resource discovery logic

Changing 'InitializationResourcesDecorator' -> 'ResourcesDecorator' - Niklas Nielsen On Sept. 21, 2015, 7:14 p.m., Felix Abecassis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https:/

Re: Review Request 38750: Updated changelog for 0.25.0

Remoortere. Repository: mesos Description --- Updated changelog for 0.25.0 Diffs (updated) - CHANGELOG fc48cc2aafbfa5342d68401426f246685a5fbef6 Diff: https://reviews.apache.org/r/38750/diff/ Testing --- Thanks, Niklas Nielsen

Re: Review Request 38750: Updated changelog for 0.25.0

--------- On Sept. 24, 2015, 10:13 p.m., Niklas Nielsen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38750/ > --

Re: Review Request 38963: Added initial draft of networking user-doc.

nt158802> Kill double spaces :) docs/networking.md (line 251) <https://reviews.apache.org/r/38963/#comment158803> NOTE: docs/networking.md (line 253) <https://reviews.apache.org/r/38963/#comment158804> s/sets /sets it's/? docs/networking.md (line 260) <https://reviews.a

Re: Review Request 38963: Added initial draft of networking user-doc.

- On Oct. 2, 2015, 11:08 a.m., Kapil Arya wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38963/ > --- > > (Updat

Re: Review Request 37903: stout: Fix bug in IPNetwork::create() with zero prefix.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37903/#review100848 --- I'll get this in today :) Thanks Neil! - Niklas Nielsen On Sept

Review Request 38822: Added default upgrade steps to 0.24.X to 0.25.X section

', and Vinod Kone. Repository: mesos Description --- See summary Diffs - docs/upgrades.md 8e33b9b90654a43a452f97fc66e969ecb0f3d6dd Diff: https://reviews.apache.org/r/38822/diff/ Testing --- Thanks, Niklas Nielsen

Re: Review Request 38749: Added `image_providers` flags to configuration.md.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38749/#review100527 --- Ship it! Ship It! - Niklas Nielsen On Sept. 24, 2015, 9:57 p.m

Review Request 38750: Updated changelog for 0.25.0

Description --- Updated changelog for 0.25.0 Diffs - CHANGELOG 18af16785ca969740bd0eb5e1dee985e2609dfb2 Diff: https://reviews.apache.org/r/38750/diff/ Testing --- Thanks, Niklas Nielsen

Re: Review Request 38634: Added Systemd environment check to LinuxLauncher.

ply, visit: > https://reviews.apache.org/r/38634/ > --- > > (Updated Sept. 22, 2015, 6:48 p.m.) > > > Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Niklas > Nielsen, Thomas Rampelberg, and Timothy Chen. > > > Bugs: MESOS-3425 > https://is

Re: Review Request 38575: Added masterSlaveLostHook

masterSlaveLostHook test Thanks, Niklas Nielsen

Re: Review Request 38637: Added recovery warnings for LinuxLauncher on Systemd.

- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38637/ > ----------- > > (Updated Sept. 22, 2015, 6:48 p.m.) > > > Review request for mesos, Benjamin Hindman, Artem Harutyunya

Re: Review Request 38637: Added recovery warnings for LinuxLauncher on Systemd.

behavior, how to setup systemd's policies etc? - Niklas Nielsen On Sept. 22, 2015, 10:29 a.m., Joris Van Remoortere wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 38635: Create and Start `mesos_executor.slice` in LinuxLauncher on Systemd.

tps://reviews.apache.org/r/38635/#comment157225> Wouldn't the idomatic way be to use stout's file abstractions? - Niklas Nielsen On Sept. 22, 2015, 10:29 a.m., Joris Van Remoortere wrote: > > --- > This is an automatically g

Re: Review Request 38517: Make attributes.hpp public

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38517/#review100069 --- Ship it! Ship It! - Niklas Nielsen On Sept. 21, 2015, 6:26 p.m

Re: Review Request 38574: Fixed race in hook self-message loop and reenabled VerifySlaveLaunchExecutorHook test

/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 Diff: https://reviews.apache.org/r/38574/diff/ Testing --- make check (gtest_repeat=100 and gtest_shuffle=1) Thanks, Niklas Nielsen

Review Request 38575: Added masterSlaveLostHook

/ Testing --- make check with new masterSlaveLostHook test Thanks, Niklas Nielsen

Review Request 38574: Fixed race in hook self-message loop and reenabled VerifySlaveLaunchExecutorHook test

b23a587c683c391ca860b3b7d876902987f4d158 Diff: https://reviews.apache.org/r/38574/diff/ Testing --- make check (gtest_repeat=100 and gtest_shuffle=1) Thanks, Niklas Nielsen

Re: Review Request 38575: Added masterSlaveLostHook

src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 Diff: https://reviews.apache.org/r/38575/diff/ Testing --- make check with new masterSlaveLostHook test Thanks, Niklas Nielsen

Re: Review Request 38279: Enabled resources/attributes discovery

> On Sept. 14, 2015, 1:20 p.m., Niklas Nielsen wrote: > > Per Connors comment, we need a test to exercise the new code Ping - let's get this in soon :) - Niklas --- This is an automatically generated e-mail. To reply, vis

Re: Review Request 38363: Refactored container Launcher to accept namespaces dynamically.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38363/#review99281 --- Ship it! Ship It! - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

tps://reviews.apache.org/r/38370/#comment156192> Was "Docker.NetworkSettings.IPAddress" in 0.24.0? If so, don't we want to deprecate this over a release cycle? (Supporting both in the interim) - Niklas Nielsen On Sept. 15, 2015, 2:11 p.m.,

Re: Review Request 38366: Added helper to model Labels message for state.json.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38366/#review99282 --- Ship it! Ship It! - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

--- On Sept. 15, 2015, 2:11 p.m., Kapil Arya wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38370/ > ----------- > > (Updat

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

the fields we want custom formatting for? include/mesos/mesos.proto (line 1392) <https://reviews.apache.org/r/38367/#comment156205> s/upto/up to/ - Niklas Nielsen On Sept. 15, 2015, 7:05 p.m., Kapil Arya

Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38368/#review99334 --- Ship it! Ship It! - Niklas Nielsen On Sept. 16, 2015, 3 p.m

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

src/slave/slave.cpp (line 4380) <https://reviews.apache.org/r/38253/#comment156234> Great! Let's include the container id's as well for debugging :) - Niklas Nielsen On Sept. 15, 2015, 6:08 a.

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

> On Sept. 14, 2015, 11:07 a.m., Niklas Nielsen wrote: > > src/tests/oversubscription_tests.cpp, line 859 > > <https://reviews.apache.org/r/38253/diff/1/?file=1067134#file1067134line859> > > > > Mind adding a TODO about doing a full blown object comparison?

Re: Review Request 38366: Added helper to model Labels message for state.json.

that we do this to avoid the nested 'labels: labels: [ ... ]' by just using JSON::protobuf()? src/common/protobuf_utils.cpp (line 140) <https://reviews.apache.org/r/38366/#comment155883> This could break existing 3rd party parsing; why not leave it set? - Niklas Nielsen On Sept. 14

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

olation include/mesos/mesos.proto (line 1390) <https://reviews.apache.org/r/38367/#comment155680> pop? :) - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote: > > --- > This is an automatically generated e-

Re: Review Request 38367: Added NetworkInfo message to ContainerInfo and TaskStatus.

ent155695> Quite a long piece of straight line code; mind throwing in some comments breaking it up a bit and explaining what you are doing? - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote: > > --- > This is

Re: Review Request 38364: Minor refactor for MesosContainerizer.

830 - 833) <https://reviews.apache.org/r/38364/#comment155700> How about moving this up as well? - Niklas Nielsen On Sept. 14, 2015, 2:54 p.m., Kapil Arya wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 38366: Added helper to model Labels message for state.json.

() to support Labels instead? - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 38364: Minor refactor for MesosContainerizer.

description? Hard to tell from the current title and description. - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 38363: Refactored container Launcher to accept namespaces dynamically.

ou get here? - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 38368: Replaced slaveTaskStatusLabelDecorator hook with slaveTaskStatusDecorator hook.

(line 513) <https://reviews.apache.org/r/38368/#comment155663> Mind preceeding this test code blob with a comment about the setup? - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote: > > --- > This is

Re: Review Request 38365: Updated Isolator::prepare to return list of required namespaces.

tps://reviews.apache.org/r/38365/#comment155698> Taken we still wrap comments at 70, this needs to be wrapped :) - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote: > > --- > This is an automatically generated e

Re: Review Request 38279: Enabled resources/attributes discovery

about 'InitialResourcesDecorator' or something that indicates that it's during initialization. src/slave/slave.cpp (line 383) <https://reviews.apache.org/r/38279/#comment155628> newline - Niklas Nielsen On Sept. 14, 2015, 10:39 a.m., Felix Abe

Re: Review Request 38370: Updated docker executor to set container IP in TaskStatus::NetworkInfo.

to '3rd party tools' Is there a corresponding test we could change to verify that this works as expected? - Niklas Nielsen On Sept. 14, 2015, 1:55 p.m., Kapil Arya wrote: > > --- > This is an automatically generated e-mail. To rep

Re: Review Request 38343: mesos: Fixed punctuation in log message.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38343/#review98878 --- Ship it! Ship It! - Niklas Nielsen On Sept. 13, 2015, 10:09 p.m

Re: Review Request 38253: Add containerId to ResourceUsage to enable QoS controller to target a container

r/to the QoS Controller/ src/tests/oversubscription_tests.cpp (line 859) <https://reviews.apache.org/r/38253/#comment15> Mind adding a TODO about doing a full blown object comparison? - Niklas Nielsen On Sept.

Re: Review Request 37445: Fix typos in style guide.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37445/#review98306 --- Ship it! Ship It! - Niklas Nielsen On Aug. 13, 2015, 2:58 p.m

Re: Review Request 37824: mesos: Updates for google-glog upgrade to 0.3.4

> On Aug. 27, 2015, 1:27 p.m., Mesos ReviewBot wrote: > > Bad patch! > > > > Reviews applied: [37823] > > > > Failed command: ./support/apply-review.sh -n -r 37823 > > > > Error: > > 2015-08-27 20:25:09 URL:https://reviews.apache.org/r/37823/diff/raw/ > > [9017/9017] -> "37823.patch" [1] >

Re: Review Request 37903: stout: Fix bug in IPNetwork::create() with zero prefix.

ld have been: IP(prefix == 0 ? 0 : 0x << (32 - prefix)) Will let you decide what is easier to read. I like it in terms of not writing IPNetwork twice - Niklas Nielsen On Aug. 28, 2015, 1:02 p.m.,

Re: Review Request 38231: Fixed typos in modules documentation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38231/#review98276 --- Ship it! Ship It! - Niklas Nielsen On Sept. 9, 2015, 11:51 a.m

Re: Review Request 37908: Silence oversubscription logging

, this patch moves log lines for empty resources to VLOG(1). Diffs (updated) - src/slave/slave.cpp 8cfad7b86675b10c98524174975828bbcf47aed2 Diff: https://reviews.apache.org/r/37908/diff/ Testing --- make check - with and without the patch, verifying the log statements. Thanks, Niklas

Review Request 37908: Silence oversubscription logging

Diff: https://reviews.apache.org/r/37908/diff/ Testing --- make check - with and without the patch, verifying the log statements. Thanks, Niklas Nielsen

Re: Review Request 33208: Delete detector in MesosSchedulerDriver::stop

On June 15, 2015, 12:07 p.m., Niklas Nielsen wrote: Hey Robert; BenH helped out and wrote a PoC patch here https://reviews.apache.org/r/35405 In short; it is not safe to delete the detector at this point. The patch above does it in join() and has a good descriptive block of comment

Re: Review Request 37479: Move QoS plug-ins to a specified folder like resource_estimator

) https://reviews.apache.org/r/37479/#comment151476 Same comments as above - Niklas Nielsen On Aug. 14, 2015, 6:44 p.m., Guangya Liu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 34361: converted hard-coded strings to consts

constants, which we like to avoid if unnecessary). Also, each test is ideally independent, why does the master's test for labels affect the slave's test for labels? Let's say I need a test with 5 labels, you want to have 5x2=10 global constants to address this? Niklas Nielsen wrote

Re: Review Request 35405: Enable deleting MasterDetector in MesosSchedulerDriver::join.

? If so, let's close this for now and rethink the approach. If not, let's see if we can verify it (think a test if hard, but maybe we can come up with something). - Niklas Nielsen On Aug. 24, 2015, 11:25 a.m., Benjamin Hindman wrote

Re: Review Request 37208: Fix the spell error in help message of slave component.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37208/#review95603 --- Ship it! Ship It! - Niklas Nielsen On Aug. 16, 2015, 11:50 p.m

Re: Review Request 36867: Add labels to FrameworkInfo.

On July 27, 2015, 11:36 p.m., Adam B wrote: Great first patch. Thanks for updating FrameworkInfo on reregistration with the master too! A handful of nits in my first pass. I'll take another look once you've simplified the tests with Kapil's suggestions. Niklas Nielsen wrote

Re: Review Request 37228: Updated slave task label decorator hook to pass in ExecutorInfo.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37228/#review94588 --- Ship it! Ship It! - Niklas Nielsen On Aug. 7, 2015, 3:44 p.m

Re: Review Request 37053: Set TaskStatus.executor_id on receiving a StatusUpdate from Executor.

://reviews.apache.org/r/37053/#comment148658 What do you think about emitting a warning if the executor id is already set? It could be, that users have been abusing this field before (I am not aware of any, but it is a possibility). If not, we should comment on it in mesos.proto. - Niklas Nielsen

Re: Review Request 37007: Pass ExecutorID to task-status label decorator hook.

/r/37007/#comment148416 The executor id should be in the task status (https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L973) have you run into it not being available? - Niklas Nielsen On July 31, 2015, 9:24 p.m., Kapil Arya wrote

Re: Review Request 36867: Add labels to FrameworkInfo.

, and Niklas Nielsen. Bugs: MESOS-2841 https://issues.apache.org/jira/browse/MESOS-2841 Repository: mesos Description --- This is intended to support frameworks that want to offer capabilities to the rest of the cluster (e.g., storage or some arbitrary third-party service). We

Review Request 36862: Added metric for number of preempted executors.

/diff/ Testing --- Thanks, Niklas Nielsen

Re: Review Request 36862: Added metric for number of preempted executors.

e086817a980cf88f8ff8d539e9791051f35dfbac Diff: https://reviews.apache.org/r/36862/diff/ Testing (updated) --- make check with new checks in OversubscriptionTest.QoSCorrectionKill Thanks, Niklas Nielsen

Re: Review Request 36488: Added oversubscription user doc.

/36488/diff/ Testing --- Rendered at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Re: Review Request 36488: Added oversubscription user doc.

docs/oversubscription.md PRE-CREATION Diff: https://reviews.apache.org/r/36488/diff/ Testing --- Rendered at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Re: Review Request 36488: Added oversubscription user doc.

--- On July 17, 2015, 3:27 p.m., Niklas Nielsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36488

Re: Review Request 36488: Added oversubscription user doc.

://reviews.apache.org/r/36488/diff/ Testing --- Rendered at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Re: Review Request 36488: Added oversubscription user doc.

/oversubscription.md PRE-CREATION Diff: https://reviews.apache.org/r/36488/diff/ Testing --- Rendered at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Re: Review Request 36488: Added oversubscription user doc.

docs/oversubscription.md PRE-CREATION Diff: https://reviews.apache.org/r/36488/diff/ Testing --- Rendered at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Re: Review Request 35715: Added revocable resource state validation.

is set and revocable is set`, `isRevocable` will return true. Niklas Nielsen wrote: Hey guys - did you reach a conclusion? MPark; how can we get closure on this? - Niklas --- This is an automatically generated e-mail. To reply

Re: Review Request 34361: converted hard-coded strings to consts

constants, which we like to avoid if unnecessary). Also, each test is ideally independent, why does the master's test for labels affect the slave's test for labels? Let's say I need a test with 5 labels, you want to have 5x2=10 global constants to address this? Niklas Nielsen wrote

Re: Review Request 36488: Added oversubscription user doc.

at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Re: Review Request 36488: Added first draft of oversubscription user doc

/ Testing (updated) --- Rendered at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Re: Review Request 36488: Added oversubscription user doc.

://reviews.apache.org/r/36488/diff/ Testing --- Rendered at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Re: Review Request 36488: Added first draft of oversubscription user doc

/diff/ Testing --- Rendered at: https://github.com/nqn/mesos/blob/niklas/oversubscription-user-doc/docs/oversubscription.md Thanks, Niklas Nielsen

Review Request 36488: Added first draft of oversubscription user doc

--- Added first draft of oversubscription user doc Diffs - docs/images/oversubscription-overview.jpg PRE-CREATION docs/oversubscription.md PRE-CREATION Diff: https://reviews.apache.org/r/36488/diff/ Testing --- Thanks, Niklas Nielsen

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507/#review89243 --- Ship it! Ship It! - Niklas Nielsen On June 22, 2015, 3:03 a.m

  1   2   3   >