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

2015-01-22 Thread Vinod Kone
-CREATION Diff: https://reviews.apache.org/r/30192/diff/ Testing --- make check Thanks, Vinod Kone

Re: Kill the "internal" namespace

2015-01-26 Thread Vinod Kone
SGTM. I would suggest to first address the non-proto files before changing the proto files. On Mon, Jan 26, 2015 at 12:35 PM, Kapil Arya wrote: > Hi All, > > TLDR: We currently use "mesos::internal" namespace for almost everything > inside src/. However, in most cases, it is directly enclosing

Re: Review Request 30289: Update post-reviews to support >= 0.6

2015-01-26 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30289/#review69700 --- Ship it! Ship It! - Vinod Kone On Jan. 26, 2015, 10:40 p.m

Re: Review Request 30288: Add deprecation warning to CHANGELOG regarding /stats.json

2015-01-26 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30288/#review69702 --- Ship it! Ship It! - Vinod Kone On Jan. 26, 2015, 10:38 p.m

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

2015-01-26 Thread Vinod Kone
in a subsequent review. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30192/#review69454 --- On Jan. 23, 2015, 12:46 a.m., Vinod

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

2015-01-26 Thread Vinod Kone
Diff: https://reviews.apache.org/r/30192/diff/ Testing --- make check Thanks, Vinod Kone

Review Request 30297: Used Clock instead of Stopwatch in limiter tests for reliablity.

2015-01-26 Thread Vinod Kone
--- Used Clock instead of Stopwatch in limiter tests for reliablity. Diffs - 3rdparty/libprocess/src/tests/limiter_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/30297/diff/ Testing --- Ran the limiter tests 100 times Thanks, Vinod Kone

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

2015-01-26 Thread Vinod Kone
... > > return ...; > > } > > ... > > ``` > > > > First one seems a bit easier to read? done. - Vinod --- This is an automatically generated e-mail.

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

2015-01-27 Thread Vinod Kone
ATION Diff: https://reviews.apache.org/r/30192/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-01-27 Thread Vinod Kone
s. Diffs - 3rdparty/libprocess/include/process/future.hpp 0326b23cdd475c6e86b33f9b4c63136fdecab443 Diff: https://reviews.apache.org/r/30348/diff/ Testing --- make check Thanks, Vinod Kone

Re: Review Request 30191: Moved RateLimiter tests into its own file.

2015-01-27 Thread Vinod Kone
akefile.am 6ab9cb8100163272126d533df47afac612769b76 3rdparty/libprocess/src/tests/limiter_tests.cpp PRE-CREATION 3rdparty/libprocess/src/tests/process_tests.cpp 3bbfe0a7a65acb52d139fda81816acf305d891f5 Diff: https://reviews.apache.org/r/30191/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-01-27 Thread Vinod Kone
/limiter_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/30192/diff/ Testing --- make check Thanks, Vinod Kone

Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.

2015-01-28 Thread Vinod Kone
8:49 a.m., Till Toenshoff wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > --- > > (Updated Jan. 26, 2015, 8:49 a.m.) > > > Review req

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

2015-01-28 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30352/#review70045 --- Ship it! Ship It! - Vinod Kone On Jan. 28, 2015, 3:06 a.m., Ben

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

2015-01-28 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30353/#review70047 --- Ship it! Ship It! - Vinod Kone On Jan. 28, 2015, 3:06 a.m., Ben

Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.

2015-01-28 Thread Vinod Kone
thentication and thus fall into a retry loop? - Vinod Kone On Jan. 26, 2015, 8:49 a.m., Till Toenshoff wrote: > > --- > This is an automatically generated e-mail.

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

2015-01-28 Thread Vinod Kone
don't clear the callback, the future will never get gc'ed. See bug for details. Diffs (updated) - 3rdparty/libprocess/include/process/future.hpp 0326b23cdd475c6e86b33f9b4c63136fdecab443 Diff: https://reviews.apache.org/r/30348/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-01-28 Thread Vinod Kone
reviews.apache.org/r/30348/#review70067 ------- On Jan. 28, 2015, 10 p.m., Vinod Kone wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30348/ > ---

Re: Review Request 30297: Used Clock instead of Stopwatch in limiter tests for reliablity.

2015-01-28 Thread Vinod Kone
sting --- Ran the limiter tests 100 times Thanks, Vinod Kone

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

2015-01-29 Thread Vinod Kone
g/r/30402/#comment115317> Can you add a comment here on why you do a max() here for future readers? - Vinod Kone On Jan. 29, 2015, 4:35 a.m., Ben Mahler wrote: > > --- > This is an automatically generated e-mail. To reply,

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

2015-01-29 Thread Vinod Kone
tps://reviews.apache.org/r/30423/#comment115382> I think this could be moved up, right after driver2.start() ? - Vinod Kone On Jan. 29, 2015, 7:54 p.m., Ben Mahler wrote: > > --- > This is an automatically generated e-mail. To reply,

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

2015-01-29 Thread Vinod Kone
tps://reviews.apache.org/r/30432/#comment115388> Maybe just say the same thing that was said in the earlier test (#910) for consistency? // Pause the clock to avoid scheduler registration retries. - Vinod Kone On Jan. 29, 2015, 11:13 p.m., Ben Mahler

Re: Review Request 29890: Refactored allocator interface to support general implementations.

2015-01-30 Thread Vinod Kone
:( - Vinod Kone On Jan. 30, 2015, 3:33 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 30439: Increased the default AWAIT timeout.

2015-01-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30439/#review70391 --- Unable to load the diff? - Vinod Kone On Jan. 30, 2015, 2:12 a.m

Re: Review Request 30463: Increased the default AWAIT timeout.

2015-01-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30463/#review70420 --- Ship it! Ship It! - Vinod Kone On Jan. 30, 2015, 7:54 p.m., Ben

Re: Review Request 29890: Refactored allocator interface to support general implementations.

2015-01-30 Thread Vinod Kone
che.org/r/29890/#comment115678> This comment should be inside the else block. src/tests/mesos.hpp <https://reviews.apache.org/r/29890/#comment115686> why does T have to be a HierarchicalDRFAllocator instead of an Allocator or MesosAllocator? - Vinod Kone On Jan. 30, 20

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

2015-02-02 Thread Vinod Kone
master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 Diff: https://reviews.apache.org/r/30511/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-02-02 Thread Vinod Kone
the new tests 100 times Thanks, Vinod Kone

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

2015-02-02 Thread Vinod Kone
/30514/diff/ Testing --- make check Ran the new tests 100 times Thanks, Vinod Kone

Re: Review Request 30518: fix reviewboard setting so all users have same rbt settings

2015-02-02 Thread Vinod Kone
te "support/reviewboardrc". That gets installed during bootstrap phase. - Vinod Kone On Feb. 2, 2015, 6:56 p.m., Jake Farrell wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 30512: Fixed DiskUsageCollectorTest.SymbolicLink so that it works on both ext3 and ext4.

2015-02-02 Thread Vinod Kone
? - Vinod Kone On Feb. 2, 2015, 6:38 p.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30512/ > --- >

Re: Review Request 30512: Fixed DiskUsageCollectorTest.SymbolicLink so that it works on both ext3 and ext4.

2015-02-02 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30512/#review70612 --- Ship it! Ship It! - Vinod Kone On Feb. 2, 2015, 7:48 p.m., Jie

Re: Review Request 30518: MESOS-2313: Fix reviewboard setting so all users have same rbt settings

2015-02-02 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30518/#review70615 --- Ship it! Ship It! - Vinod Kone On Feb. 2, 2015, 7:34 p.m., Jake

Re: Build failed in Jenkins: mesos-reviewbot #3838

2015-02-02 Thread Vinod Kone
circular dep in "depends on". fixed it. On Mon, Feb 2, 2015 at 11:46 AM, Apache Jenkins Server < jenk...@builds.apache.org> wrote: > See > > -- > [...truncated 8486 lines...] > File "./support/verify_r

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

2015-02-02 Thread Vinod Kone
yes, i'm going to keep suggesting it) it's actually a shared_ptr. fixed. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30514/#review70602 -----

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

2015-02-02 Thread Vinod Kone
isReady()) { > > // Shutdown the slave. > > } else if (future.isFailed()) { > > // Not expected to happen? > > } else if (future.isDiscarded()) { > > // Do nothing here, maybe print a message. > > } > > > >

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

2015-02-02 Thread Vinod Kone
iff: https://reviews.apache.org/r/30514/diff/ Testing --- make check Ran the new tests 100 times Thanks, Vinod Kone

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

2015-02-03 Thread Vinod Kone
orks'. No functional changes. Diffs (updated) - src/master/master.hpp 337e00aa46ea127f3667e3383d631c3fb8e22f30 src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 Diff: https://reviews.apache.org/r/30511/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-02-03 Thread Vinod Kone
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30514/#review70768 ------- On Feb. 3, 2015, 2:39 a.m., Vinod Kone wrote: > >

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

2015-02-03 Thread Vinod Kone
amespace process? done - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30514/#review70776 --- On Feb. 3, 2015, 2:39 a.m., Vinod Kone wrote: > > --

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

2015-02-03 Thread Vinod Kone
ache.org/r/30514/diff/ Testing --- make check Ran the new tests 100 times Thanks, Vinod Kone

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

2015-02-03 Thread Vinod Kone
> > ``` > > And we can add parser for Rate. I'll add a TODO for now. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30514/#review70775 --- On Feb. 3, 2015,

Re: Review Request 30513: Added validation for CREATE offer operation.

2015-02-03 Thread Vinod Kone
- > > (Updated Feb. 2, 2015, 6:40 p.m.) > > > Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone. > > > Bugs: MESOS-2100 > https://issues.apache.org/jira/browse/MESOS-2100 > > > Repository: mesos > > > Desc

Re: Review Request 30513: Added validation for CREATE offer operation.

2015-02-03 Thread Vinod Kone
g/r/30513/#comment116254> Include resource.name() for better debugging. src/master/validation.cpp <https://reviews.apache.org/r/30513/#comment116255> ditto. - Vinod Kone On Feb. 2, 2015, 6:40 p.m., Jie Yu wrote: > > --

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

2015-02-03 Thread Vinod Kone
org/r/29742/#comment116257> We didn't do "isempty" above, so how about getting rid of "is" as prefix? I think returning a "bool" signals a "is". - Vinod Kone On Jan. 29, 2015, 5:27 a.m., Michael Park wrote: > > ---

Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.

2015-02-03 Thread Vinod Kone
ps://reviews.apache.org/r/27760/#comment116269> Can this be authenticating[pid].discard() ? src/master/master.cpp <https://reviews.apache.org/r/27760/#comment116270> ditto. - Vinod Kone On Feb. 3, 2015, 11:05 a.m., Till Toenshoff wrote: > > -

Review Request 30584: Added metrics for slave shutdowns.

2015-02-03 Thread Vinod Kone
956fe5042d7478d90d4f03059e248694ba2b95e5 Diff: https://reviews.apache.org/r/30584/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-02-03 Thread Vinod Kone
k with killing it altogether, if that's confusing. LMK. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30584/#review70855 -----

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

2015-02-03 Thread Vinod Kone
src/master/master.cpp 10056861b95ed9453c971787982db7d09f09f323 src/master/metrics.hpp 6a43abc914dce24c60b5db57ee01d172c8258e82 src/master/metrics.cpp 956fe5042d7478d90d4f03059e248694ba2b95e5 Diff: https://reviews.apache.org/r/30584/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-02-03 Thread Vinod Kone
#x27;. No functional changes. Diffs (updated) - src/master/master.hpp cd37ee9d3c57bcd91f08cd402ec1e4a09d9e42ee src/master/master.cpp d04b2c4041d8fe8978b877f07579a6f907903e1b Diff: https://reviews.apache.org/r/30511/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-02-03 Thread Vinod Kone
/ Testing --- make check Ran the new tests 100 times Thanks, Vinod Kone

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

2015-02-03 Thread Vinod Kone
d04b2c4041d8fe8978b877f07579a6f907903e1b src/master/metrics.hpp 6a43abc914dce24c60b5db57ee01d172c8258e82 src/master/metrics.cpp 956fe5042d7478d90d4f03059e248694ba2b95e5 Diff: https://reviews.apache.org/r/30584/diff/ Testing --- make check Thanks, Vinod Kone

Re: When does scheduler driver send the LaunchTasksMessage

2015-02-03 Thread Vinod Kone
On Tue, Feb 3, 2015 at 5:46 AM, Chengwei Yang wrote: > As we can see, mesos-master send offer to chronos at 18:32:33, but > received all > 4 decline message (LaunchTasksMessage) at 18:33.03, we are very curious > why the > first decline doesn't sent before sleep 30 seconds? > Just to be sure, if

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

2015-02-03 Thread Vinod Kone
e7e2af63da785644f3f7e6e23607c02be962a2c6 Diff: https://reviews.apache.org/r/30514/diff/ Testing --- make check Ran the new tests 100 times Thanks, Vinod Kone

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

2015-02-03 Thread Vinod Kone
d04b2c4041d8fe8978b877f07579a6f907903e1b src/master/metrics.hpp 6a43abc914dce24c60b5db57ee01d172c8258e82 src/master/metrics.cpp 956fe5042d7478d90d4f03059e248694ba2b95e5 Diff: https://reviews.apache.org/r/30584/diff/ Testing --- make check Thanks, Vinod Kone

Re: When does scheduler driver send the LaunchTasksMessage

2015-02-04 Thread Vinod Kone
You are understanding is right. The driver is single threaded. So, if it is busy in a callback it can't send a message to the master. On Tue, Feb 3, 2015 at 7:04 PM, Chengwei Yang wrote: > On Tue, Feb 03, 2015 at 05:21:55PM -0800, Vinod Kone wrote: > > > > On Tue, Fe

Re: Review Request 30513: Added validation for CREATE offer operation.

2015-02-04 Thread Vinod Kone
ere validate returns None? - Vinod Kone On Feb. 4, 2015, 1:10 a.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

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

2015-02-04 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30635/#review71033 --- Ship it! Ship It! - Vinod Kone On Feb. 4, 2015, 7:38 p.m., Jie

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

2015-02-04 Thread Vinod Kone
ill METRICS_SNAPSHOT macro. would recommend doing this part as its own review. src/tests/slave_tests.cpp <https://reviews.apache.org/r/27531/#comment116559> ditto. rephrase comment as suggested above. - Vinod Kone On Jan. 20, 2015, 9:26 p.m., Dominic Hamon wrote: > > --

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

2015-02-04 Thread Vinod Kone
If a rate limit is specified schedule the shutdown according // to the rate limit. LOG(INFO) << "Scheduling shutdown of slave " << slaveId << " due to health check timeout"; shuttingDown = limiter.g

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

2015-02-04 Thread Vinod Kone
t a too bad thing in this case as the logic is > more clear IMO. > > Jie Yu wrote: > Another option to avoid dup code is: > > 1) s/shutdown/scheduleShutdown > 2) let 'shutdown' be the actual shutdown (sending message) > > Ben Mahler wrote: >

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

2015-02-04 Thread Vinod Kone
/ Testing --- make check Ran the new tests 100 times Thanks, Vinod Kone

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

2015-02-04 Thread Vinod Kone
69b945df58c173322b95aeae9cf4ed70329c4bb3 src/master/metrics.hpp 6a43abc914dce24c60b5db57ee01d172c8258e82 src/master/metrics.cpp 956fe5042d7478d90d4f03059e248694ba2b95e5 Diff: https://reviews.apache.org/r/30584/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-02-04 Thread Vinod Kone
hink we want to inject a rate limiter at some point? yea, was thinking about that. will add a TODO. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30514/#review71059 --- On Feb. 4, 201

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

2015-02-04 Thread Vinod Kone
69b945df58c173322b95aeae9cf4ed70329c4bb3 src/master/metrics.hpp 6a43abc914dce24c60b5db57ee01d172c8258e82 src/master/metrics.cpp 956fe5042d7478d90d4f03059e248694ba2b95e5 Diff: https://reviews.apache.org/r/30584/diff/ Testing --- make check Thanks, Vinod Kone

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

2015-02-04 Thread Vinod Kone
/ Testing --- make check Ran the new tests 100 times Thanks, Vinod Kone

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

2015-02-04 Thread Vinod Kone
tions of out-of-order messages? Does the master reconcile them? I guess if master fails over we cannot reconcile. Do we expect Frameworks to reconcile in this case based on the offers they get? - Vinod Kone On Feb. 4, 2015, 7:13 p.m., Jie Yu wrote: > > --

Re: Review Request 30678: Fix unsigned/signed comparison in master_tests

2015-02-05 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30678/#review71253 --- Ship it! Ship It! - Vinod Kone On Feb. 5, 2015, 5:47 p.m

Re: Review Request 30682: Changed default disk quota check interval to be 15 seconds.

2015-02-05 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30682/#review71260 --- Ship it! Ship It! - Vinod Kone On Feb. 5, 2015, 6:30 p.m., Jie

Re: Review Request 29890: Refactored allocator interface to support general implementations.

2015-02-05 Thread Vinod Kone
> On Jan. 31, 2015, 1:05 a.m., Vinod Kone wrote: > > src/tests/mesos.hpp, line 676 > > <https://reviews.apache.org/r/29890/diff/7/?file=841772#file841772line676> > > > > why does T have to be a HierarchicalDRFAllocator instead of an > > Allocator or M

Re: Review Request 29925: Moved allocation related sources into a separate directory.

2015-02-05 Thread Vinod Kone
947> How about this structure for consistency: master/allocator/allocator.hpp master/allocator/mesos/hierarchical.hpp master/allocator/sorter/sorter.hpp master/allocator/sorter/drf/sorter.hpp - Vinod Kone On Feb. 4, 2015, 4:49 p.m., Alexander Rukletsov

Re: Review Request 29927: Removed unnecessary lifecycle method from MasterAllocatorTest.

2015-02-05 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29927/#review71292 --- Ship it! Ship It! - Vinod Kone On Feb. 4, 2015, 4:50 p.m

Re: Review Request 29929: Cleaned up includes in allocation sources.

2015-02-05 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29929/#review71293 --- empty src/master/allocation/mesos/hierarchical.hpp ? - Vinod Kone

Re: Review Request 29931: Extracted MesosAllocator into a separate file.

2015-02-05 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29931/#review71296 --- empty hierarchical.hpp? - Vinod Kone On Feb. 4, 2015, 4:53 p.m

Re: Review Request 29932: Renamed test allocator actions for consistency.

2015-02-05 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29932/#review71302 --- Ship it! Ship It! - Vinod Kone On Feb. 4, 2015, 4:55 p.m

Re: Review Request 30082: Cleaned up namespace hierarchy in allocation sources.

2015-02-05 Thread Vinod Kone
d out. - Vinod Kone On Feb. 4, 2015, 4:54 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

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

2015-02-05 Thread Vinod Kone
lt;https://reviews.apache.org/r/27531/#comment116998> I would put "strings::lower(TaskState_Name(state)) + "/"" on next line for readability. - Vinod Kone On Feb. 5, 2015, 6:07 p.m., Dominic Hamon wrote: > > --- >

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

2015-02-06 Thread Vinod Kone
> On Feb. 4, 2015, 8:41 p.m., Vinod Kone wrote: > > src/tests/master_authorization_tests.cpp, lines 365-373 > > <https://reviews.apache.org/r/27531/diff/4/?file=768126#file768126line365> > > > > It is a bit weird to see a test for these metrics in authori

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

2015-02-06 Thread Vinod Kone
>tasks_failed; break; case TASK_KILLED: ++metrics->tasks_killed; break; case TASK_LOST: ++metrics->tasks_lost; break; default: break; } } } ``` src/master/metrics.hpp <https://reviews.apache.org/r/27531/#comment117211>

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

2015-02-06 Thread Vinod Kone
> On Feb. 6, 2015, 6:56 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 3601 > > <https://reviews.apache.org/r/27531/diff/7/?file=852243#file852243line3601> > > > > Looks like you missed updating metrics here? > > > > Now that I t

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

2015-02-06 Thread Vinod Kone
> On Feb. 6, 2015, 6:56 p.m., Vinod Kone wrote: > > src/master/master.cpp, line 3601 > > <https://reviews.apache.org/r/27531/diff/7/?file=852243#file852243line3601> > > > > Looks like you missed updating metrics here? > > > > Now that I t

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

2015-02-06 Thread Vinod Kone
531/#comment117233> don't need this if condition because we are inside if (terminated). - Vinod Kone On Feb. 6, 2015, 6:42 p.m., Dominic Hamon wrote: > > --- > This is an automatically generated e-mail. To r

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

2015-02-08 Thread Vinod Kone
period depends on command executor grace period. - Vinod Kone On Feb. 4, 2015, 2:14 a.m., Ben Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http

Re: Review Request 29925: Moved allocation related sources into a separate directory.

2015-02-08 Thread Vinod Kone
> On Feb. 5, 2015, 7:13 p.m., Vinod Kone wrote: > > src/Makefile.am, lines 466-469 > > <https://reviews.apache.org/r/29925/diff/6/?file=848172#file848172line466> > > > > How about this structure for consistency: > > > > master/allocator

Re: Scaling Proposal: MAINTAINERS Files

2015-02-08 Thread Vinod Kone
+1 @vinodkone > On Feb 8, 2015, at 4:04 AM, Tom Arnfeld wrote: > > This sounds really interesting Ben - I'm definitely +1 to the idea. > > > > > The only question that comes up in my mind is, are files/areas of the code > base segmented enough at the moment for this to be useful? > > >

Re: Review Request 30761: docs incorrectly state permissive mode ACLs are string-valued when they are actually boolean-valued

2015-02-08 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30761/#review71602 --- Ship it! Ship It! - Vinod Kone On Feb. 7, 2015, 11:59 p.m., R.B

Re: Review Request 29883: Added /master/slaves endpoint.

2015-02-09 Thread Vinod Kone
883/#comment117519> s/receives/shows/ src/master/http.cpp <https://reviews.apache.org/r/29883/#comment117521> can you just call model(slave) instead? src/tests/master_tests.cpp <https://reviews.apache.org/r/29883/#comment117518> s/actuall/actual/ - Vinod Kone On Feb.

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

2015-02-09 Thread Vinod Kone
atically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30580/ > --- > > (Updated Feb. 4, 2015, 2:14 a.m.) > > > Review request for mesos, Alexander Rukletsov, Niklas Nielsen, and Vinod Kone. > > > Repos

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

2015-02-09 Thread Vinod Kone
ent117536> s/If/When/ src/tests/persistent_volume_tests.cpp <https://reviews.apache.org/r/28809/#comment117538> s/registration/re-registration/ - Vinod Kone On Feb. 9, 2015, 5:59 p.m., Jie Yu wrote: > > ---

Re: Scaling Proposal: MAINTAINERS Files

2015-02-09 Thread Vinod Kone
I like MAINTAINERS because it sounds less authoritative than OWNERS. FWIW, maintainers is also a well understood and well used term (e.g: https://www.kernel.org/doc/linux/MAINTAINERS, https://wiki.jenkins-ci.org/display/JENKINS/Hosting+Plugins#HostingPlugins-AddingMaintainerInformation ) On Sun,

Re: Review Request 30508: Prepare persistent volumes in slave.

2015-02-09 Thread Vinod Kone
508/#comment117555> also, add a check for persistence? - Vinod Kone On Feb. 9, 2015, 6 p.m., Jie Yu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 30510: Allowed Mesos containerizer to prepare and update volumes.

2015-02-09 Thread Vinod Kone
e cleaned up? src/slave/containerizer/mesos/containerizer.cpp <https://reviews.apache.org/r/30510/#comment117558> Add a comment here, - Vinod Kone On Feb. 9, 2015, 6 p.m., Jie Yu wrote: > > --- > This is an autom

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

2015-02-09 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30580/#review71694 --- Ship it! Ship It! - Vinod Kone On Feb. 4, 2015, 2:14 a.m., Ben

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

2015-02-09 Thread Vinod Kone
ws.apache.org/r/30583/#comment117564> you can just do new TestContainerizer(executor, flags) once you make the above suggested change. src/tests/mesos.cpp <https://reviews.apache.org/r/30583/#comment117565> ditto. - Vinod Kone On Feb. 4, 2015

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

2015-02-09 Thread Vinod Kone
g/r/30601/#comment117566> Can this comment be expanded on what this means? src/tests/slave_tests.cpp <https://reviews.apache.org/r/30601/#comment117578> Why do we need 2 expectations here? - Vinod Kone On Feb. 5, 2015, 9:02 p.m., Ben

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

2015-02-09 Thread Vinod Kone
7580> Just say NOTE: We only track metrics sources and reasons for terminal states. src/master/metrics.cpp <https://reviews.apache.org/r/27531/#comment117581> indent by 2 spaces. - Vinod Kone On Feb. 9, 2015, 10:

Re: Review Request 29883: Added /master/slaves endpoint.

2015-02-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29883/#review71796 --- Ship it! Ship It! - Vinod Kone On Feb. 10, 2015, 11:33 a.m

Re: Review Request 30508: Prepare persistent volumes in slave.

2015-02-10 Thread Vinod Kone
tps://reviews.apache.org/r/30508/#comment117682> hmm. can we use a Clock::pause() and Clock::settle() here to ensure that the CheckpointResourcesMessage is processed? - Vinod Kone On Feb. 9, 2015, 10:37 p.m., Jie Yu wrote: > > ---

Re: Review Request 30509: Added executor working directory to Container struct in Mesos containerizer.

2015-02-10 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30509/#review71825 --- Ship it! Ship It! - Vinod Kone On Feb. 9, 2015, 6 p.m., Jie Yu

Re: Review Request 30510: Allowed Mesos containerizer to prepare and update volumes.

2015-02-10 Thread Vinod Kone
ps://reviews.apache.org/r/30510/#comment117687> Log a warning here? src/slave/containerizer/mesos/containerizer.cpp <https://reviews.apache.org/r/30510/#comment117688> Log a warning here? - Vinod Kone On Feb.

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