and how to
change it to be more meaningful I think it won't be unreasonable to just delete
it for now. We can always add it in its new form back.
- Jiang Yan Xu
On Nov. 22, 2019, noon, Xudong Ni wrote:
>
> ---
> This is an
as partitioned after prolonged ZK disconnection than to silently drop
messages.
```
src/slave/slave.cpp
Lines 6419 (patched)
<https://reviews.apache.org/r/71742/#comment306510>
"registered master" is not a valid concept here, let's say "because the
master can
> On Aug. 2, 2019, 3:17 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp
> > Line 570 (original), 566 (patched)
> > <https://reviews.apache.org/r/71193/diff/3/?file=2158799#file2158799line570>
> >
> > You are not chang
> On Aug. 2, 2019, 4:03 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/disk.cpp
> > Lines 424 (patched)
> > <https://reviews.apache.org/r/71194/diff/3/?file=2158813#file2158813line424>
> >
> > FWIW sandbox scanning predat
gt;
Same question: why don't we fail here?
- Jiang Yan Xu
On Aug. 1, 2019, 7:27 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71192/#review217060
---
Ship it!
Ship It!
- Jiang Yan Xu
On July 31, 2019, 6:03 p.m
ed by an `else` to avoid another level
of nesting.
src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Line 807 (original)
<https://reviews.apache.org/r/71193/#comment304306>
We could move the logging about the `dir` up above `erase` if we still want
to log it (maybe for `VLOG(1)`)?
pp
Lines 243 (patched)
<https://reviews.apache.org/r/71192/#comment304300>
Nit: with initializers I don't think we wrap elements with spaces.
- Jiang Yan Xu
On July 31, 2019, 6:03 p.m., James Peach wrote:
>
> ---
> This
he value is not meaningful and we are just using the
map to simulate a "bounded hashset".
- Jiang Yan Xu
On July 18, 2019, 8:10 p.m., Xudong Ni wrote:
>
> ---
> This is an automatically generated e-mail. T
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70862/#review215928
---
Ship it!
Ship It!
- Jiang Yan Xu
On June 16, 2019, 7:13 p.m
> On June 5, 2019, 12:16 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 516 (patched)
> > <https://reviews.apache.org/r/70741/diff/2/?file=2147412#file2147412line524>
> >
> > Is it more explicit if you just name the ope
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70741/#review215714
---
Ship it!
Ship It!
- Jiang Yan Xu
On June 5, 2019, 1:44 a.m
entially
implementing how we use a `mv` command?
- Jiang Yan Xu
On May 28, 2019, 9:29 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit
g path = 2;
```
make more sense?
- Jiang Yan Xu
On May 23, 2019, 11:46 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70660/#review215327
---
Ship it!
Ship It!
- Jiang Yan Xu
On May 16, 2019, 7:24 p.m
t this patch) and see
that
```
# ls -l /proc/1/
```
shows the files under it are all owned by root, which does appear to mean
that the process' dumpable flag is not 1 according to
http://man7.org/linux/man-pages/man5/proc.5.html?
- Jiang Yan Xu
On Feb. 8, 2019, 1:
ent297762>
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.
---
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
> 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
-mail. To reply, visit:
> https://reviews.apache.org/r/68706/
> -----------
>
> (Updated Oct. 19, 2018, 4:56 p.m.)
>
>
> Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.
>
>
> Bugs: MESO
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
> 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
> 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.apac
2. There are 0 recovered agents (e.g., 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
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
---
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
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
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:
> h
e.org/documentation/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 i
---
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
---
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
> 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
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.
---
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
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
ts 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
> 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
https://github.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:
>
> ---
which would 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:
>
> --
171 (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 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()`
ing
---
Make check.
Thanks,
Jiang Yan Xu
/reconciliation_tests.cpp b06244b5f3efad3a88e367cd8e26cebd3d9f2e43
Diff: https://reviews.apache.org/r/66769/diff/1/
Testing
---
Make check.
Thanks,
Jiang Yan Xu
"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
emove 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
.
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. 1
Would it be possible to hoist 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
nerated e-mail. To reply, visit:
> 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.
>
isit:
https://reviews.apache.org/r/64516/#review195041
-----------
On Jan. 10, 2018, 2:23 p.m., Jiang Yan Xu wrote:
>
> ---
> This is an automatically generate
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
/
Changes: https://reviews.apache.org/r/64515/diff/3-4/
Testing
---
make check.
Thanks,
Jiang Yan Xu
utomatically 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://rev
very involved change, 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
>
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 M
olumes 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., Jie Yu wrote:
>
> --
---
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
> 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
isit:
https://reviews.apache.org/r/64940/#review194794
-------
On Jan. 3, 2018, 5:35 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
940/#comment273784>
If you wait for the ping, 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
---
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
7f4b9ed938fd478fdf750b464c531bf76de7d798
Diff: https://reviews.apache.org/r/64933/diff/1/
Testing
---
make check. Ran the affected test for 1000 iterations.
Thanks,
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
rated e-mail. To reply, visit:
https://reviews.apache.org/r/64515/#review193690
---
On Jan. 3, 2018, 11 a.m., Jiang Yan Xu wrote:
>
> ---
> This is an automatically
/
Changes: https://reviews.apache.org/r/64515/diff/2-3/
Testing
---
make check.
Thanks,
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:
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
---
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
/
Testing
---
Used a markdown viewer.
Thanks,
Jiang Yan Xu
aster/master.cpp b10d0341276090bfa70aaa4fd6317a560e3334ea
Diff: https://reviews.apache.org/r/64514/diff/1/
Testing
---
make check.
Thanks,
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
us is from the 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
---
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
tedly` here is probably racy due to
```
EXPECT_CALL(sched, statusUpdate(&driver, _))
.WillOnce(FutureArg<1>(&reconcileUpdate));
```
below, not to mention the case could be interesting to test.
We can set
not, we can 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 generate
> 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
> 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
> 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()`
--
> 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,
``?
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
270794>
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
sos.proto
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 automatica
> 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
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
/reviews.apache.org/r/61473/
> -----------
>
> (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/j
> 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
uld we 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 s
://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 `fore
l.cpp
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
---
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:
>
> -
/.
Thanks,
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()`?
>
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
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/
> -
Thanks,
Jiang Yan Xu
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:
>
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:
>
> ---
---
make check.
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
Diff: https://reviews.apache.org/r/63831/diff/1/
Testing
---
make check.
Added a test.
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
1 - 100 of 1075 matches
Mail list logo
