#comment255362>
Put it above declaration?
src/slave/gc.cpp
Lines 194-195 (patched)
<https://reviews.apache.org/r/53479/#comment255363>
No need for passing both. In fact, `infos` is not used here when we should:
there could be other entries of the same timeout added while the
sn't look like we need to set this.
src/tests/slave_recovery_tests.cpp
Lines 2765 (patched)
<https://reviews.apache.org/r/56895/#comment255429>
Strictly speaking this is EXPECT_*
src/tests/slave_recovery_tests.cpp
Lines 2779 (patched)
<https://reviews.apache.org/r/56895/#com
s.cpp b4d7791782a5d9e10fa44489057c8504d594bab2
3rdparty/libprocess/src/tests/process_tests.cpp
c6109547d04bd06e9395e4ec5f5130fd1500f9a0
Diff: https://reviews.apache.org/r/60821/diff/1/
Testing
---
make check.
Thanks,
Jiang Yan Xu
-
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60821/#review180383
-----------
On July 12, 2017, 5:04 p.m., Jiang Yan Xu wrote:
>
> ---
/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
src/tests/reconciliation_tests.cpp 9c36b9a0558ab56ce45fdb93189a1b99f487ba4d
Diff: https://reviews.apache.org/r/60854/diff/1/
Testing
---
make check.
Thanks,
Jiang Yan Xu
in the `slaves.recovered`.
Diffs
-
src/master/master.cpp 33eca0d17459781fdc2ea915e8f40c78dd306962
Diff: https://reviews.apache.org/r/60400/diff/1/
Testing (updated)
---
make check.
Will add more tests.
Thanks,
Jiang Yan Xu
p;
src/slave/gc.cpp
Lines 210 (patched)
<https://reviews.apache.org/r/53479/#comment255738>
Use `CHECK_EQ(timeouts.erase(info->path), 1);`?
- Jiang Yan Xu
On July 13, 2017, 2:48 p.m., Jacob Janco wrote:
>
> ---
> On July 11, 2017, 6:47 p.m., Jiang Yan Xu wrote:
> > src/slave/gc.cpp
> > Lines 179 (patched)
> > <https://reviews.apache.org/r/53479/diff/4/?file=1774291#file1774291line184>
> >
> > Better to call `onAny` and then `CHECK` that it's always ready
`
> > UPID and force them to explicitly pass the anonymous UPID. This makes the
> > used of anonymous message sends explicit which I think is preferable to
> > this implicit approach.
>
> Jiang Yan Xu wrote:
> Here's my thinking:
>
> - In li
`
> > UPID and force them to explicitly pass the anonymous UPID. This makes the
> > used of anonymous message sends explicit which I think is preferable to
> > this implicit approach.
>
> Jiang Yan Xu wrote:
> Here's my thinking:
>
> - In li
035db18db3a64a9e358c1c54cc18a4bdeb85d8bf
Diff: https://reviews.apache.org/r/60898/diff/1/
Testing
---
make check.
Thanks,
Jiang Yan Xu
check.
Thanks,
Jiang Yan Xu
pp
Line 100 (original), 119 (patched)
<https://reviews.apache.org/r/53479/#comment255950>
`s/(unsigned long)1/1u/`
src/slave/gc.cpp
Lines 209 (patched)
<https://reviews.apache.org/r/53479/#comment255951>
`s/(unsigned long)1/1u/`
- Jiang Yan Xu
On July 14, 2017, 6:23
> On July 17, 2017, 1:43 p.m., Jiang Yan Xu wrote:
> > src/slave/gc.hpp
> > Lines 114-117 (original), 115-118 (patched)
> > <https://reviews.apache.org/r/53479/diff/8/?file=1777247#file1777247line116>
> >
> > I see that `promise == that.promise` here is
`
> > UPID and force them to explicitly pass the anonymous UPID. This makes the
> > used of anonymous message sends explicit which I think is preferable to
> > this implicit approach.
>
> Jiang Yan Xu wrote:
> Here's my thinking:
>
> - In li
> On July 17, 2017, 1:43 p.m., Jiang Yan Xu wrote:
> > src/slave/gc.hpp
> > Lines 114-117 (original), 115-118 (patched)
> > <https://reviews.apache.org/r/53479/diff/8/?file=1777247#file1777247line116>
> >
> > I see that `promise == that.promise` here is
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60925/#review180813
---
Ship it!
Committing with some minor edits.
- Jiang Yan Xu
`
> >
> > :)
Sure :)
- Jiang Yan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60917/#review180773
------------
/master.cpp d895154c00eef17511c46ddd0b75922f952707eb
Diff: https://reviews.apache.org/r/60400/diff/3/
Changes: https://reviews.apache.org/r/60400/diff/2-3/
Testing
---
make check.
Will add more tests.
Thanks,
Jiang Yan Xu
r/60791/#comment256172>
Why await?
src/slave/containerizer/fetcher_process.hpp
Lines 147-148 (patched)
<https://reviews.apache.org/r/60791/#comment256173>
If these are used only for monitoring, should we just follow the convention
mentioned
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60936/#review180873
---
Ship it!
Ship It!
- Jiang Yan Xu
On July 17, 2017, 7:15 p.m
an the fact that some callback has been called.
- Jiang Yan Xu
On July 18, 2017, 3:15 p.m., Jacob Janco wrote:
>
> ---
> This is an automatically generated e-mail. To reply,
`
> > UPID and force them to explicitly pass the anonymous UPID. This makes the
> > used of anonymous message sends explicit which I think is preferable to
> > this implicit approach.
>
> Jiang Yan Xu wrote:
> Here's my thinking:
>
> - In li
ove):
```
// There should be no registrar operation across both agent termination
// and reregistration.
EXPECT_CALL(*master.get()->registrar.get(), apply(_))
.Times(0);
```
- Jiang Yan
---
This is an automatically generated e-m
Jiang Yan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60898/#review180894
-----------
On July 16, 2017, 11:29 a.m., Jiang Yan Xu w
has
been marked unreachable.
Diffs (updated)
-
src/tests/slave_tests.cpp e1cc96dbb279aea998b99779ee1b55e96fee4e41
Diff: https://reviews.apache.org/r/60898/diff/3/
Changes: https://reviews.apache.org/r/60898/diff/2-3/
Testing
---
make check.
Thanks,
Jiang Yan Xu
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60956/#review181019
---
Ship it!
Ship It!
- Jiang Yan Xu
On July 18, 2017, 8:04 p.m
> On July 18, 2017, 2:16 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/fetcher.cpp
> > Lines 272-275 (patched)
> > <https://reviews.apache.org/r/60791/diff/3/?file=1778079#file1778079line272>
> >
> > Aside from styling/convention, would thi
---
Minor fix to use the more idiomatic `send` method.
Diffs
-
src/log/network.hpp 635079fd7e42f3da3a595208c15f288d768424f7
Diff: https://reviews.apache.org/r/61018/diff/1/
Testing
---
make check
Thanks,
Jiang Yan Xu
mesos/containerizer.cpp
9376d14d66f5dc7e91c7c0e9da253f5eb9347539
Diff: https://reviews.apache.org/r/61089/diff/1/
Testing
---
make check
Thanks,
Jiang Yan Xu
Yan Xu
> On July 18, 2017, 2:16 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/fetcher.cpp
> > Lines 272-275 (patched)
> > <https://reviews.apache.org/r/60791/diff/3/?file=1778079#file1778079line272>
> >
> > Aside from styling/convention, would thi
after this patch lands.
- Jiang Yan Xu
On July 18, 2017, 8:05 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache
rg/r/61089/#review181472
-------
On July 24, 2017, 2:48 p.m., Jiang Yan Xu wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache
r.hpp
ea0691945d3450fcc336df03d9cf7b48afbd8b3d
src/slave/containerizer/mesos/containerizer.cpp
9376d14d66f5dc7e91c7c0e9da253f5eb9347539
Diff: https://reviews.apache.org/r/61089/diff/2/
Changes: https://reviews.apache.org/r/61089/diff/1-2/
Testing
---
make check
Thanks,
Jiang Yan Xu
/master.cpp e12c997dad04f8a4ddb47a993a84b2b05c9e2f32
Diff: https://reviews.apache.org/r/61193/diff/1/
Testing
---
make check and tested in a testing cluster.
Thanks,
Jiang Yan Xu
t the flag can now be
used more widely?
CHANGELOG
Lines 17 (patched)
<https://reviews.apache.org/r/61220/#comment257373>
The flag is `--no-enforce_container_disk_quota`.
- Jiang Yan Xu
On July 28, 2017, 9:01 a.m., James
a strong case for an
abstraction, provide it as a more general helper (possibly put it where
`JSON::Object Metrics()` is defined).
- Jiang Yan Xu
On July 31, 2017, 9:31 a.m., James Peach wrote:
>
> ---
> This is an autom
)
<https://reviews.apache.org/r/61220/#comment257638>
Remove the `#`. This is why we also need "Testing Done" for doc changes. :)
Normally we just say "Tested with a markdown viewer".
- Jiang Yan Xu
On July 31, 2017, 2:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61368/#review181929
---
Ship it!
Ship It!
- Jiang Yan Xu
On Aug. 1, 2017, 5:59 p.m
(patched)
<https://reviews.apache.org/r/61367/#comment257754>
4 spaces?
- Jiang Yan Xu
On Aug. 1, 2017, 5:59 p.m., Michael Park wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://re
(original), 4809-4811 (patched)
<https://reviews.apache.org/r/61368/#comment257755>
4 space indentation?
- Jiang Yan Xu
On Aug. 1, 2017, 5:59 p.m., Michael Park wrote:
>
> ---
> This is an automatically generated e
aster.cpp
Lines 7132-7137 (patched)
<https://reviews.apache.org/r/61473/#comment258590>
Do this only when we actually send an update i.e., `framework->connected()`?
src/master/master.cpp
Lines 8928-8931 (original), 8907-8910 (patched)
<https://reviews.apache.org/r/61473/#comment
doesn't apply cleanly anymore due to the
delay...
CHANGELOG
Lines 29-31 (patched)
<https://reviews.apache.org/r/61220/#comment259384>
The list of features are orderer by their JIRA number. Reorder it?
- Jiang Yan Xu
On Aug. 16, 2017, 3:37 p.m., James
(patched)
<https://reviews.apache.org/r/61620/#comment259390>
`onAny` takes a `void(const Future&)` callback so this looks odd, even
thought it probably compiles due to the defer wrapper.
Since this has a `then` continuation below, can we just put it there?
- Jiang Yan Xu
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61620/#review183374
---
Ship it!
Ship It!
- Jiang Yan Xu
On Aug. 21, 2017, 12:10
s added here?
- Jiang Yan Xu
On Aug. 10, 2017, 3 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61260/
> --
>
> (Updated July 31, 2017, 9:31 a.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-7842
> https://issues.apache.org/jira/browse/MESOS-7842
>
>
> Repository: mesos
>
>
> Description
> ---
>
&g
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61124/#review183412
---
Ship it!
Ship It!
- Jiang Yan Xu
On July 25, 2017, 4:26 p.m
are metrics from the `Slave` actor which is not
true.
- Jiang Yan Xu
On Aug. 22, 2017, 8:51 a.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://rev
)
<https://reviews.apache.org/r/61261/#comment259512>
Will have to adjust the docs according to the suggested names in r/61260/
Here and below.
- Jiang Yan Xu
On Aug. 22, 2017, 8:53 a.m., James Peach wrote:
>
> -
> On Aug. 22, 2017, 11 a.m., Jiang Yan Xu wrote:
> > src/slave/gc.cpp
> > Lines 53 (patched)
> > <https://reviews.apache.org/r/61260/diff/3/?file=1801592#file1801592line53>
> >
> > Actually, it didn't occur to me earlier but a
)
<https://reviews.apache.org/r/61261/#comment259552>
Fix ordering?
- Jiang Yan Xu
On Aug. 22, 2017, 3:31 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://re
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61892/#review183800
---
Ship it!
Ship It!
- Jiang Yan Xu
On Aug. 24, 2017, 12:34
(original), 1007 (patched)
<https://reviews.apache.org/r/61893/#comment259847>
Clean up the comment a bit:
"*that* the path removal *that* failed" doesn't sound right?
- Jiang Yan Xu
On Aug. 24, 2017, 12:34 p.
as clean as it could be. Of course I understand we'll refactor this later
anyway.
- Jiang Yan Xu
On Aug. 24, 2017, 1:07 a.m., Michael Park wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
>
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61661/#review183975
---
Ship it!
Ship It!
- Jiang Yan Xu
On Aug. 15, 2017, 7:48 a.m
t restart before reregistration completes.
```
- Jiang Yan Xu
On Aug. 15, 2017, 7:48 a.m., Ilya Pronin wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https:
s.apache.org/r/61473/#comment260159>
This comment is probably unncessary because "partition aware +
non-partition aware" pretty much means "we don't care" so why comment about it
:)
src/master/master.cpp
Lines 8948-8950 (patched)
<https://reviews.apache.org/r/61473/#c
tps://reviews.apache.org/r/61690/#comment260192>
I can add a TODO here...
// TODO: Share the default ZK session timeout across master and agent.
- Jiang Yan Xu
On Aug. 25, 2017, 10:05 a.m., Ilya Pronin wrote:
>
> ---
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61965/#review184127
---
Ship it!
Ship It!
- Jiang Yan Xu
On Aug. 29, 2017, 5:51 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61689/#review184131
---
Ship it!
Ship It!
- Jiang Yan Xu
On Aug. 25, 2017, 10:05
s we really just have a few of these
types).
- Jiang Yan Xu
On July 19, 2017, 2:02 a.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://rev
be Prometheus' rationale as well) need a sum?
What useful thing can we do about it? This is related to the abstract concept
of "counting semantics" that we need to document in a design. We need a
whiteboard discussion offline but perhaps we should write it down for other
reviewer
--
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61125/
> -----------
>
> (Updated July 25, 2017, 4:57 p.m.)
>
>
> Review request for mesos, Benjamin
patch precisely to avoid dispatching onto the gc actor
itself because the rmdirs could block new task launches.
- Jiang Yan Xu
On Sept. 11, 2017, 2:39 p.m., Chun-Hung Hsiao wrote:
>
> ---
> This is an automatically generated e-mail.
generic?
e.g.,
```
// For executing path removals in a separate thread.
```
- Jiang Yan Xu
On Sept. 12, 2017, 1:52 p.m., Chun-Hung Hsiao wrote:
>
> ---
> This is an automatically generated
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62230/#review186341
---
Ship it!
Ship It!
- Jiang Yan Xu
On Sept. 26, 2017, 3:04
t263403>
This is going to send `TASK_UNREACHABLE` to the operator API subscribers
even for NPA framework tasks.
We should probably be consistent and send `TASK_LOST`.
- Jiang Yan Xu
On Sept. 24, 2017, 3:46 p.m., Megha Sharma wrote:
>
> ---
comments?
src/tests/partition_tests.cpp
Lines 2045 (patched)
<https://reviews.apache.org/r/61473/#comment263581>
Perhaps add a comment: `// The agent may resend status updates.`
- Jiang Yan Xu
On Sept. 24, 2017, 3:46 p.m., Megha Sharma wrote:
>
> ---
---
Ran this test for 100 iterations.
Thanks,
Jiang Yan Xu
iew187830
---
On Oct. 12, 2017, 11:34 a.m., Jiang Yan Xu wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://revie
Count/MasterFailover_BENCHMARK_Test.AgentReregistrationDelay/2
(109941 ms)
[--] 3 tests from AgentFrameworkTaskCount/MasterFailover_BENCHMARK_Test
(258497 ms total)
```
The recently patches cut down the time by nearly 50%. These were built with
`--enable-optimize`.
I can also get some flame graphs.
Thanks,
Jiang Yan Xu
5b6efe5faa3c3b10f1f714f582a155b368f8ccaf
Diff: https://reviews.apache.org/r/63175/diff/1/
Testing
---
make check.
I didn' write a new test as the externally observable behavior doesn't change.
Thanks,
Jiang Yan Xu
tomatically 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:
>
> -
tests from 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
force the 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 th
`. For now
let's follow 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
```
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
/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., Zhit
)
<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.,
---
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
s
in 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,
---
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
ter 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
ently 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
e-run-queue --enable-lock-free-event-queue
--enable-last-in-first-out-fixed-size-semaphore`.
Thanks,
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
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 Yan X
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:
>
> -
with /r/63830/.
Thanks,
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
Diff: https://reviews.apache.org/r/63831/diff/1/
Testing
---
make check.
Added a test.
Thanks,
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
---
make check.
Thanks,
Jiang Yan Xu
This is an automatically 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:
>
> ---
was equivalent 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:
>
Thanks,
Jiang Yan Xu
https://reviews.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/
> -
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
801 - 900 of 1075 matches
Mail list logo
