Re: Review Request 69615: Disable containerizer ptrace attach.

2019-01-18 Thread Jiang Yan Xu
; If disallow is the default, it's a breaking change right? Can the default be allowing it? - Jiang Yan Xu On Jan. 2, 2019, 9:15 a.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 69373: Replaced a log consensus `CHECK()` with CHECK_GE()`.

2018-11-16 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69373/#review210613 --- Ship it! Ship It! - Jiang Yan Xu On Nov. 16, 2018, 11:25

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-11-02 Thread Jiang Yan Xu
> On Oct. 18, 2018, 11:14 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 1874-1875 (patched) > > <https://reviews.apache.org/r/68706/diff/5/?file=2098490#file2098490line1874> > > > > If we use equality operator and only set the timer

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-11-02 Thread Jiang Yan Xu
--- > > (Updated Oct. 19, 2018, 4:56 p.m.) > > > Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu. > > > Bugs: MESOS-9178 > https://issues.apache.org/jira/browse/MESOS-9178 >

Re: Review Request 69233: Fixed a GetContainers crash when using XFS disk isolation.

2018-11-01 Thread Jiang Yan Xu
Lines 660 (patched) <https://reviews.apache.org/r/69233/#comment294910> Might be worth adding a comment about root disks not having `Source` since people often forget. :) - Jiang Yan Xu On Nov. 1, 2018, 1:33 p.m., James Peach

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-10-29 Thread Jiang Yan Xu
> On Oct. 18, 2018, 11:14 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 1874-1875 (patched) > > <https://reviews.apache.org/r/68706/diff/5/?file=2098490#file2098490line1874> > > > > If we use equality operator and only set the timer

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-10-18 Thread Jiang Yan Xu
> On Oct. 18, 2018, 11:14 a.m., Jiang Yan Xu wrote: > > Sorry for the delay. Feel free to chat if it's not clear! - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.or

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-10-18 Thread Jiang Yan Xu
brand new cluster), should all of the metrics be zero or non-existent? I feel like they should be zero, as in, e.g., 100% of all 0 agents are reregistered within 0 secs. So timer handles this natrually. Also it sets the `_secs` name for you but that's a minor conveninence. - Jiang Yan X

Re: Review Request 68586: Add the output file to the hash on CommandInfo::URI.

2018-10-17 Thread Jiang Yan Xu
tps://reviews.apache.org/r/68586/#comment294257> So `cache` is counted here. Does it mean that if I specify two duplicate URIs with different `cache` values one will overwrite another in fetching? - Jiang Yan Xu On Aug. 31, 2018, 9:47 a.m., James Peach

Re: Review Request 68586: Add the output file to the hash on CommandInfo::URI.

2018-10-17 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68586/#review209706 --- Ship it! Ship It! - Jiang Yan Xu On Aug. 31, 2018, 9:47 a.m

Re: Review Request 68587: Fixed fetcher deadlock with duplicate URIs.

2018-10-17 Thread Jiang Yan Xu
src/slave/containerizer/fetcher.cpp Lines 421 (patched) <https://reviews.apache.org/r/68587/#comment294252> Using `newEntries.at()` is more idiomatic even though we did check that the entry exists. - Jiang Yan Xu On Aug. 31, 2018, 9:47 a.m., James

Re: Review Request 68999: Fixed the XFS project ID labeling to not cross mount points.

2018-10-12 Thread Jiang Yan Xu
same filesystem (which `FTS_XDEV` doesn't give us)? - Jiang Yan Xu On Oct. 11, 2018, 3:23 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https:

Re: Review Request 68706: Added master failover reregistration progress metrics.

2018-09-13 Thread Jiang Yan Xu
mentation/latest/monitoring/#registrar) has the same meaning, therefore it's OK to drop old values which may reduce precision but will not changing the meaning to the statistics. Let's start with how the monitoring system will use it and work backwards. I think my proposal in https://issues.apa

Re: Review Request 68448: Skip the container if there's no container network.

2018-08-21 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68448/#review207693 --- Ship it! Ship It! - Jiang Yan Xu On Aug. 21, 2018, 12:16

Re: Review Request 67365: Added MESOS-8340 to the 1.7.x CHANGELOG.

2018-05-30 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67365/#review204104 --- Ship it! Ship It! - Jiang Yan Xu On May 30, 2018, 11:57 a.m

Re: Review Request 67365: Added MESOS-8340 to the 1.7.x CHANGELOG.

2018-05-30 Thread Jiang Yan Xu
> On May 30, 2018, 10:56 a.m., Jiang Yan Xu wrote: > > docs/upgrades.md > > Lines 445 (patched) > > <https://reviews.apache.org/r/67365/diff/1/?file=2031690#file2031690line445> > > > > Is it TCP specific? > > James Peach wrote: > Yes. I s

Re: Review Request 67365: Added MESOS-8340 to the 1.7.x CHANGELOG.

2018-05-30 Thread Jiang Yan Xu
org/r/67365/#comment286451> Is it TCP specific? - Jiang Yan Xu On May 29, 2018, 3:46 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Review Request 67115: Added more diagnostic info for a CHECK.

2018-05-14 Thread Jiang Yan Xu
--- Added more diagnostic info for a CHECK. Diffs - src/master/allocator/mesos/hierarchical.cpp 1000968be6a2935a4cac571414d7f06d7df7acf0 Diff: https://reviews.apache.org/r/67115/diff/1/ Testing --- make check Thanks, Jiang Yan Xu

Re: Review Request 66919: Failure to update registry should abort the master process.

2018-05-07 Thread Jiang Yan Xu
on the commit message (which comes from the review summary). src/master/http.cpp Lines 4163 (patched) <https://reviews.apache.org/r/66919/#comment284492> You missed a space between `result)` and `{` which I didn't catch initially but fixed up in a subsequent commit. - Jiang Yan Xu On May 7

Re: Review Request 66919: Failure to update registry should abort the master process.

2018-05-04 Thread Jiang Yan Xu
rite tests for but we can test it manually without checking in the code. e.g., I changed one test a bit to show the check failure: https://github.com/xujyan/mesos/commit/68051320a87431f6d2f3fbad6b0b97814200a731 Could you update the "Testing Done" section with the link? That should cover it

Re: Review Request 66919: Failure to update registry should abort the master process.

2018-05-04 Thread Jiang Yan Xu
> On May 4, 2018, 8:19 a.m., Jiang Yan Xu wrote: > > src/master/http.cpp > > Lines 4163-4165 (patched) > > <https://reviews.apache.org/r/66919/diff/1/?file=2016039#file2016039line4163> > > > > The following will be more idiomatic. > > > &g

Re: Review Request 66919: Failure to update registry should abort the master process.

2018-05-04 Thread Jiang Yan Xu
com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md which in turn references https://google.github.io/styleguide/cppguide.html - Jiang Yan Xu On May 3, 2018, 9:43 a.m., Xudong Ni wrote: > > --- > This

Re: Review Request 66644: Remove unknown unreachable tasks when agent reregisters.

2018-05-03 Thread Jiang Yan Xu
d be less jagged. src/tests/partition_tests.cpp Lines 396 (patched) <https://reviews.apache.org/r/66644/#comment284230> Period to end the sentence. - Jiang Yan Xu On May 1, 2018, 10:18 p.m., Megha Sharma wrote: > > ---

Re: Review Request 66644: Remove unknown unreachable tasks when agent reregisters.

2018-05-01 Thread Jiang Yan Xu
(patched) <https://reviews.apache.org/r/66644/#comment283967> This seems to be exceeded the 80 character limit. src/tests/partition_tests.cpp Lines 47 (patched) <https://reviews.apache.org/r/66644/#comment283968> It doesn't look like this is used? - Jiang Yan Xu On Apri

Re: Review Request 66644: Remove unknown unreachable tasks when agent reregisters.

2018-05-01 Thread Jiang Yan Xu
> On April 25, 2018, 3:39 p.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 10577-10584 (patched) > > <https://reviews.apache.org/r/66644/diff/3/?file=2012148#file2012148line10577> > > > > This is the case we don't need to use `at()` a

Review Request 66778: Fixed Master which generated updates with the latest task state.

2018-04-24 Thread Jiang Yan Xu
ing --- Make check. Thanks, Jiang Yan Xu

Review Request 66769: Fixed flaky ReconciliationTest.ReconcileStatusUpdateTaskState.

2018-04-23 Thread Jiang Yan Xu
/reconciliation_tests.cpp b06244b5f3efad3a88e367cd8e26cebd3d9f2e43 Diff: https://reviews.apache.org/r/66769/diff/1/ Testing --- Make check. Thanks, Jiang Yan Xu

Re: Review Request 65967: Eliminated some copying in the master's Accept call handler path.

2018-03-08 Thread Jiang Yan Xu
into a "copy elimination" epic? Completing a sweep is a large scale effort so an Epic with subtasks helps tracking remaining work? - Jiang Yan Xu On March 7, 2018, 7:52 p.m., Benjamin Mahler wrote: > > --- > This is an automatical

Review Request 65438: Fixed flaky SlaveTest.RegisteredAgentReregisterAfterFailover.

2018-01-31 Thread Jiang Yan Xu
registrar operation when the test asserts there's none. Diffs - src/tests/slave_tests.cpp 20b874481d3818574731fc30ba9df1fc2bcbe900 Diff: https://reviews.apache.org/r/65438/diff/1/ Testing --- Ran the test with 1000 iterations. Thanks, Jiang Yan Xu

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-12 Thread Jiang Yan Xu
. src/master/master.cpp Lines 10937-10941 (original), 10935-10939 (patched) <https://reviews.apache.org/r/64940/#comment274514> Given our latest conclusion let's revert this back to ``` count += framework->unreachableTasks.size(); ``` - Jiang Yan Xu On Jan. 12, 20

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-12 Thread Jiang Yan Xu
oist the Clock::pause() statement? src/tests/partition_tests.cpp Lines 2527 (patched) <https://reviews.apache.org/r/64940/#comment274475> Remove this debugging log. src/tests/partition_tests.cpp Lines 2532-2534 (patched) <https://reviews.apache.org/r/64940/#comment274476>

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-11 Thread Jiang Yan Xu
t: > https://reviews.apache.org/r/64940/ > --- > > (Updated Jan. 5, 2018, 11:37 a.m.) > > > Review request for mesos, Benjamin Mahler, Gaston Kleiman, Jie Yu, Vinod > Kone, and Jiang Yan Xu. > > > Bugs: MESOS-8337 &

Re: Review Request 64516: Improved documentation on resource reservations.

2018-01-10 Thread Jiang Yan Xu
https://reviews.apache.org/r/64516/#review195041 ----------- On Jan. 10, 2018, 2:23 p.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-m

Re: Review Request 64516: Improved documentation on resource reservations.

2018-01-10 Thread Jiang Yan Xu
fa1f309f2b770e37964af2d5599519875195dbec Diff: https://reviews.apache.org/r/64516/diff/2/ Changes: https://reviews.apache.org/r/64516/diff/1-2/ Testing --- Used a markdown viewer. Thanks, Jiang Yan Xu

Re: Review Request 64515: Used `reserve_resources` ACL for static reservations.

2018-01-10 Thread Jiang Yan Xu
/ Changes: https://reviews.apache.org/r/64515/diff/3-4/ Testing --- make check. Thanks, Jiang Yan Xu

Re: Review Request 64515: Used `reserve_resources` ACL for static reservations.

2018-01-10 Thread Jiang Yan Xu
ly generated e-mail. To reply, visit: https://reviews.apache.org/r/64515/#review195042 ----------- On Jan. 10, 2018, 2:22 p.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apach

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-09 Thread Jiang Yan Xu
, I would recommend > > keeping what you have here but with a giant TODO (your current comment in > > #10058 doesn't go into enough detail about the complexity here) that talks > > about the above stuff. Your change at least keeps the parity with the > > (broken) semantics

Re: Review Request 64968: Removed the inaccurate `slaves.transitioning` function in the master.

2018-01-09 Thread Jiang Yan Xu
tps://reviews.apache.org/r/64968/#comment274241> Perhaps it'll be more informative to log the `slaveId` if it is present? i.e., distinguish the two sub-cases in logging? - Jiang Yan Xu On Jan. 4, 2018, 5:38 p.m., Benjamin Mahler

Re: Review Request 65050: Added some defensive checks for persistent volume sharing check.

2018-01-09 Thread Jiang Yan Xu
e volumes on disk. Docker containerizer has the same `if (current.contains(resource))` check but that check doesn't work because we set `container->resources` first there. - Jiang Yan Xu On Jan. 9, 2018, 12:44 p.m.,

Re: Review Request 65049: Fixed the persistent volume permission issue in DockerContainerizer.

2018-01-09 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65049/#review195068 --- Ship it! Ship It! - Jiang Yan Xu On Jan. 9, 2018, 11:31 a.m

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-05 Thread Jiang Yan Xu
> On Jan. 4, 2018, 2:09 p.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 10061 (patched) > > <https://reviews.apache.org/r/64940/diff/1/?file=1930130#file1930130line10062> > > > > Perhaps move all the logic around determin

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-05 Thread Jiang Yan Xu
64940/#review194794 --- On Jan. 3, 2018, 5:35 p.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

2018-01-04 Thread Jiang Yan Xu
ng, you don't need to settle? src/tests/master_tests.cpp Lines 7686 (patched) <https://reviews.apache.org/r/64940/#comment273785> No need to settle? - Jiang Yan Xu On Jan. 3, 2018, 5:35 p.m., James Peach wrote: > > ---

Re: Review Request 64939: Abort libprocess when a Process throws an uncaught exception.

2018-01-04 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64939/#review194774 --- Ship it! Ship It! - Jiang Yan Xu On Jan. 3, 2018, 5:30 p.m

Review Request 64933: Fixed flaky PartitionedSlaveReregistrationMasterFailover test.

2018-01-03 Thread Jiang Yan Xu
7f4b9ed938fd478fdf750b464c531bf76de7d798 Diff: https://reviews.apache.org/r/64933/diff/1/ Testing --- make check. Ran the affected test for 1000 iterations. Thanks, Jiang Yan Xu

Re: Review Request 64898: Removed duplicated code that tests for removable tasks.

2018-01-03 Thread Jiang Yan Xu
tps://reviews.apache.org/r/64898/#comment273641> Would it be simpler to just use the new variable without a lambda? ``` TaskState newState = latestState.getOrElse(status.state()); ``` - Jiang Yan Xu On Jan. 2, 2018, 4:26 p.m., James Peach

Re: Review Request 64515: Used `reserve_resources` ACL for static reservations.

2018-01-03 Thread Jiang Yan Xu
rg/r/64515/#review193690 --- On Jan. 3, 2018, 11 a.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://review

Re: Review Request 64515: Used `reserve_resources` ACL for static reservations.

2018-01-03 Thread Jiang Yan Xu
/ Changes: https://reviews.apache.org/r/64515/diff/2-3/ Testing --- make check. Thanks, Jiang Yan Xu

Re: Review Request 64515: Used `reserve_resources` ACL for static reservations.

2018-01-03 Thread Jiang Yan Xu
I think that's OK but in this case it's easier to understand the two cases individually with smaller and independent tests? - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://re

Re: Review Request 64514: Refactor out `authorizeReserveResources` that takes a `Resources`.

2017-12-13 Thread Jiang Yan Xu
but during agent registration. Diffs - src/master/master.hpp 232cc3758f240db626c4fdaf852163fa48af4dd7 src/master/master.cpp b10d0341276090bfa70aaa4fd6317a560e3334ea Diff: https://reviews.apache.org/r/64514/diff/1/ Testing --- make check. Thanks, Jiang Yan Xu

Re: Review Request 64098: Send status updates when agent re-registers.

2017-12-12 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64098/#review193557 --- Ship it! Ship It! - Jiang Yan Xu On Dec. 12, 2017, 4:54 a.m

Review Request 64516: Improved documentation on resource reservations.

2017-12-11 Thread Jiang Yan Xu
/ Testing --- Used a markdown viewer. Thanks, Jiang Yan Xu

Review Request 64514: Refactor out `authorizeReserveResources` that takes a `Resources`.

2017-12-11 Thread Jiang Yan Xu
/master.cpp b10d0341276090bfa70aaa4fd6317a560e3334ea Diff: https://reviews.apache.org/r/64514/diff/1/ Testing --- make check. Thanks, Jiang Yan Xu

Review Request 64515: Used `reserve_resources` ACL for static reservations.

2017-12-11 Thread Jiang Yan Xu
232cc3758f240db626c4fdaf852163fa48af4dd7 src/master/master.cpp b10d0341276090bfa70aaa4fd6317a560e3334ea src/tests/master_authorization_tests.cpp 676543a5ad1bb5d47011fc2a8b05dfaaeef18c64 Diff: https://reviews.apache.org/r/64515/diff/1/ Testing --- make check. Thanks, Jiang Yan Xu

Re: Review Request 64098: Send status updates when agent re-registers.

2017-12-11 Thread Jiang Yan Xu
agent's reregister message but I think `finsihedStatusFromMaster` vs `finsihedStatusFromAgent` is clearer). Also, can we add a comment too? e.g., the first status is sent as part of the reregistation and the second is by the agent's status update manager after it is reregsitered. - J

Re: Review Request 64429: Fixed skipping of completed frameworks in the master failover benchmark.

2017-12-07 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64429/#review193192 --- Ship it! Ship It! - Jiang Yan Xu On Dec. 7, 2017, 2:33 p.m

Re: Review Request 64098: Send status updates when agent re-registers.

2017-12-05 Thread Jiang Yan Xu
sUpdate(, _)) .WillOnce(FutureArg<1>()); ``` below, not to mention the case could be interesting to test. We can set the expectation explicitly: The first update will come from the master, th

Re: Review Request 64098: Send status updates when agent re-registers.

2017-12-04 Thread Jiang Yan Xu
an use `TEST_F_TEMP_DISABLED_ON_WINDOWS` as part of the test name to disable it. Please review all the tests in this RR for this. - Jiang Yan Xu On Dec. 1, 2017, 4:29 p.m., Megha Sharma wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

2017-12-01 Thread Jiang Yan Xu
> On Nov. 21, 2017, 10:27 a.m., Alexander Rukletsov wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Lines 430-437 (original), 431-438 (patched) > > <https://reviews.apache.org/r/63831/diff/1/?file=1894169#file1894169line431> > > > > Do w

Re: Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

2017-12-01 Thread Jiang Yan Xu
> On Nov. 21, 2017, 9:53 a.m., Alexander Rukletsov wrote: > > src/tests/scheduler_tests.cpp > > Lines 1515-1516 (original), 1534-1535 (patched) > > <https://reviews.apache.org/r/63830/diff/1/?file=1892884#file1892884line1542> > > > > `.WillRepeate

Re: Review Request 63741: Fixed a bug in devolving framework subscription with suppressed roles.

2017-12-01 Thread Jiang Yan Xu
> On Nov. 21, 2017, 10:04 a.m., Alexander Rukletsov wrote: > > src/internal/devolve.cpp > > Lines 203-204 (patched) > > <https://reviews.apache.org/r/63741/diff/1/?file=1892879#file1892879line203> > > > > I believe we prefer writing `CopyFrom()`

Re: Review Request 64250: Added new reasons task status update.

2017-12-01 Thread Jiang Yan Xu
--- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64250/ > --- > > (Updated Dec. 1, 2017, 1:06 p.m.) > > > Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan

Re: Review Request 64250: Added new reasons task status update.

2017-12-01 Thread Jiang Yan Xu
``? Also when it involves doc changes, could you in the testing done section mention that you have used a markdown viewer to review the doc change? - Jiang Yan Xu On Dec. 1, 2017, 1:06 p.m., Megha Sharma wrote: > > --- &g

Re: Review Request 64098: Send status updates when agent re-registers.

2017-12-01 Thread Jiang Yan Xu
4> s/REASON_AGENT_REREGISTERED/REASON_SLAVE_REREGISTERED/ - Jiang Yan Xu On Nov. 27, 2017, 4:55 p.m., Megha Sharma wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 64250: Added new reasons task status update.

2017-12-01 Thread Jiang Yan Xu
Lines 2387 (patched) <https://reviews.apache.org/r/64250/#comment270737> Ditto. - Jiang Yan Xu On Dec. 1, 2017, 8:20 a.m., Megha Sharma wrote: > > --- > This is an automatically genera

Re: Review Request 64098: Send status updates when agent re-registers.

2017-11-30 Thread Jiang Yan Xu
> On Nov. 28, 2017, 11:22 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 6788 (patched) > > <https://reviews.apache.org/r/64098/diff/3/?file=1902267#file1902267line6788> > > > > I think this type of status updates could benefit from a d

Re: Review Request 61473: Do not kill non partition aware tasks.

2017-11-29 Thread Jiang Yan Xu
0231> Tweak comment so it's less jagged. src/master/master.cpp Lines 9614-9616 (original), 9565-9568 (patched) <https://reviews.apache.org/r/61473/#comment270232> Reorder the comments and the CHECK. - Jiang Yan Xu On Nov. 28,

Re: Review Request 61473: Do not kill non partition aware tasks.

2017-11-28 Thread Jiang Yan Xu
- > > (Updated Nov. 28, 2017, 9:28 a.m.) > > > Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu. > > > Bugs: MESOS-7215 > https://issues.apache.org/jira/browse/MESOS-7215 > > >

Re: Review Request 64098: Send status updates when agent re-registers.

2017-11-28 Thread Jiang Yan Xu
> On Nov. 28, 2017, 11:22 a.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 6788 (patched) > > <https://reviews.apache.org/r/64098/diff/3/?file=1902267#file1902267line6788> > > > > I think this type of status updates could benefit from a d

Re: Review Request 64098: Send status updates when agent re-registers.

2017-11-28 Thread Jiang Yan Xu
e actually assign these values? ``` protobuf::getTaskHealth(*task), protobuf::getTaskCheckStatus(*task), None(), protobuf::getTaskContainerStatus(*task) ``` Note that the `None`s are default values so you don't have to specify tr

Re: Review Request 61473: Do not kill non partition aware tasks.

2017-11-28 Thread Jiang Yan Xu
//reviews.apache.org/r/61473/#comment270001> Assert this invariant inside this `if`? ``` CHECK(!unreachable) << task->task_id(); ``` src/master/master.cpp Line 10318 (original), 10271 (patched) <https://reviews.apache.org/r/61473/#comment270002> Use `foreachvalue`.

Re: Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

2017-11-27 Thread Jiang Yan Xu
5ce9ceaa3a5f84a1e076d45448863c418531cc2b src/tests/scheduler_tests.cpp 45fc9c0cfccdb22c2e3e8d5de30c04575814a0e9 Diff: https://reviews.apache.org/r/63831/diff/2/ Changes: https://reviews.apache.org/r/63831/diff/1-2/ Testing --- make check. Added a test. Thanks, Jiang Yan Xu

Re: Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

2017-11-27 Thread Jiang Yan Xu
This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63831/#review191613 --- On Nov. 15, 2017, 11:05 p.m., Jiang Yan Xu wrote: > > ---

Re: Review Request 63741: Fixed a bug in devolving framework subscription with suppressed roles.

2017-11-27 Thread Jiang Yan Xu
> On Nov. 21, 2017, 10:04 a.m., Alexander Rukletsov wrote: > > src/internal/devolve.cpp > > Lines 194-196 (original), 194-196 (patched) > > <https://reviews.apache.org/r/63741/diff/1/?file=1892879#file1892879line194> > > > > What about `evolve()`? >

Re: Review Request 63741: Fixed a bug in devolving framework subscription with suppressed roles.

2017-11-27 Thread Jiang Yan Xu
reply, visit: https://reviews.apache.org/r/63741/#review191605 ------- On Nov. 15, 2017, 10:41 a.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-mail. T

Re: Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

2017-11-27 Thread Jiang Yan Xu
eviews.apache.org/r/63830/#review191602 --- On Nov. 21, 2017, 9:46 a.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63830/ > ---

Re: Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

2017-11-21 Thread Jiang Yan Xu
Thanks, Jiang Yan Xu

Re: Review Request 63175: Do not generate UnavailableResources for inactive frameworks.

2017-11-17 Thread Jiang Yan Xu
quivalent to deactivation (note that activation has > > become a per-role thing). > > > > Mostly `connected` == `active`. But I think the point of this boolean > > is to track whether we can talk to the framework? Thoughts? > > Jiang Yan Xu wrote: >

Re: Review Request 63175: Do not generate UnavailableResources for inactive frameworks.

2017-11-17 Thread Jiang Yan Xu
matically generated e-mail. To reply, visit: https://reviews.apache.org/r/63175/#review190412 ----------- On Nov. 7, 2017, 5:02 p.m., Jiang Yan Xu wrote: > > ---

Review Request 63900: Downgraded the logging level of socket shutdown failures.

2017-11-16 Thread Jiang Yan Xu
--- make check. Thanks, Jiang Yan Xu

Review Request 63895: Fixed flaky test UnreachableAgentReregisterAfterFailover test.

2017-11-16 Thread Jiang Yan Xu
://reviews.apache.org/r/63895/diff/1/ Testing --- make check. I couldn't repro the failure locally but I think the fix makes sense logically so it probably is the fix. ;) Thanks, Jiang Yan Xu

Review Request 63831: Fixed a bug that removed the suppressed framework from sorter.

2017-11-15 Thread Jiang Yan Xu
: https://reviews.apache.org/r/63831/diff/1/ Testing --- make check. Added a test. Thanks, Jiang Yan Xu

Review Request 63830: Fixed 'NoOffersWithAllRolesSuppressed' test.

2017-11-15 Thread Jiang Yan Xu
01-00 00-00 04-00 00-00 A0-54 00-8C 7D-7F 00-00>) Expected: to be never called Actual: called once - over-saturated and active ``` The test passes with /r/63741/. Thanks, Jiang Yan Xu

Review Request 63741: Fixed framework subscription with suppressed roles.

2017-11-15 Thread Jiang Yan Xu
with /r/63830/. Thanks, Jiang Yan Xu

Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

2017-11-07 Thread Jiang Yan Xu
rc/tests/master_validation_tests.cpp Lines 4243 (patched) <https://reviews.apache.org/r/63642/#comment267727> These are `EXPECT_*`? Same below. - Jiang Yan Xu On Nov. 7, 2017, 10:47 a.m., James Peach wrote: > > -

Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

2017-11-07 Thread Jiang Yan Xu
Lines 4240-4241 (patched) <https://reviews.apache.org/r/63642/#comment267733> I was suggesting `ASSERT_*` as strictly speaking the test code prepared these, not the code to be tested so it should always succeed otherwise we should abort the test. Here and below. - Jiang

Re: Review Request 63594: Improved the fetcher exit status log message.

2017-11-06 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63594/#review190220 --- Ship it! Ship It! - Jiang Yan Xu On Nov. 6, 2017, 12:36 p.m

Re: Review Request 63174: Added a benchmark for agent reregistration during master failover.

2017-11-03 Thread Jiang Yan Xu
-queue --enable-lock-free-event-queue --enable-last-in-first-out-fixed-size-semaphore`. Thanks, Jiang Yan Xu

Re: Review Request 63174: Added a benchmark for agent reregistration during master failover.

2017-11-01 Thread Jiang Yan Xu
patches cut down the time by over 50%. These were built with `--enable-optimize --enable-lock-free-run-queue --enable-lock-free-event-queue --enable-last-in-first-out-fixed-size-semaphore`. Thanks, Jiang Yan Xu

Re: Review Request 63174: Added a benchmark for agent reregistration during master failover.

2017-11-01 Thread Jiang Yan Xu
and agent. Using `Swap` for now and will clean up after proto 3.4. - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63174/#review189093 ----------- On Nov. 1, 2017, 3:06 p.m., Jiang Yan Xu wrote: > >

Re: Review Request 63447: Fixed bulleted list formatting.

2017-10-31 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63447/#review189739 --- Ship it! Ship It! - Jiang Yan Xu On Oct. 31, 2017, 10:48

Re: Review Request 63420: Moved Linux namespace helpers into a source file.

2017-10-30 Thread Jiang Yan Xu
cpp files. Here and elsewhere in the file. - Jiang Yan Xu On Oct. 30, 2017, 10:27 a.m., James Peach wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 63332: Logged the resources in DRF sorter CHECK.

2017-10-26 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63332/#review189328 --- Ship it! Ship It! - Jiang Yan Xu On Oct. 26, 2017, 12:01

Re: Review Request 63332: Logged the resources in DRF sorter CHECK.

2017-10-26 Thread Jiang Yan Xu
) <https://reviews.apache.org/r/63332/#comment266340> Typo: s/toRemove/oldAllocation/ Could you make sure it compiles and fill out the "Testing Done" section? - Jiang Yan Xu On Oct. 26, 2017, 11:26 a.m.,

Re: Review Request 63332: Logged the resources in DRF sorter CHECK.

2017-10-26 Thread Jiang Yan Xu
/sorter.hpp Lines 350 (patched) <https://reviews.apache.org/r/63332/#comment266336> Nit: For consistency, I don't think we quote agent IDs (there are no spaces in agent IDs). Here and below? - Jiang Yan Xu On Oct. 26, 2017, 11:12 a.m., Zhitao Li

Re: Review Request 63332: Logged the resources in DRF sorter CHECK.

2017-10-26 Thread Jiang Yan Xu
It doesn't strictly belong to this review but since you are editing these lines we might as well. - Jiang Yan Xu On Oct. 26, 2017, 9:42 a.m., Zhitao Li wrote: > > --- > This is an automatically generated e-mail. To

Re: Review Request 61473: Do not kill non partition aware tasks.

2017-10-26 Thread Jiang Yan Xu
the implementation in `Master::_tasks_killing()`: check the state of each task. src/tests/partition_tests.cpp Lines 2044-2045 (patched) <https://reviews.apache.org/r/61473/#comment266298> A comment shouldn't split the chain. I think ``` .WillRepeatedly(Return()); // The

Re: Review Request 63174: Added a benchmark for agent reregistration during master failover.

2017-10-24 Thread Jiang Yan Xu
messages to go through the remote stack rather than the local > > stack. No need to think about this yet, but just something to keep in mind > > as not being accurate in this benchmark. > > Jiang Yan Xu wrote: > 1) Yeah looks like it. I used to include the setup ti

Re: Review Request 63174: Added a benchmark for agent reregistration during master failover.

2017-10-24 Thread Jiang Yan Xu
AgentFrameworkTaskCount/MasterFailover_BENCHMARK_Test (184421 ms total) ``` The recently patches cut down the time by over 50%. These were built with `--enable-optimize --enable-lock-free-run-queue --enable-lock-free-event-queue --enable-last-in-first-out-fixed-size-semaphore`. Thanks, Jiang Yan Xu

Re: Review Request 63174: Added a benchmark for agent reregistration during master failover.

2017-10-20 Thread Jiang Yan Xu
y generated e-mail. To reply, visit: https://reviews.apache.org/r/63174/#review188799 ----------- On Oct. 19, 2017, 4:28 p.m., Jiang Yan Xu wrote: > > --- > Th

  1   2   3   4   5   6   7   8   9   10   >