0105/#comment253079>
We normally in logs first state what action is taken and then attach the
error after `: `. In this case since the error has multiple lines, let's state
the action first so it's not buried by the err
> On June 23, 2017, 11:21 a.m., Jiang Yan Xu wrote:
> > src/slave/slave.cpp
> > Line 6003 (original), 6007 (patched)
> > <https://reviews.apache.org/r/60105/diff/6/?file=1759987#file1759987line6010>
> >
> > Empty line above.
>
> Megha Sharma w
ment253045>
Blank line and is the comment necessary?
Instead, you could comment on that you are capturing `offers2` in order to
get the `slaveId2` (which is what you reall want to verify)?
The same applies to `offers1`.
src/tests/slave_recovery_tests.cpp
Lines 27
---
Skipped consulting registry if the agent is in the `slaves.recovered`.
Diffs
-
src/master/master.cpp 33eca0d17459781fdc2ea915e8f40c78dd306962
Diff: https://reviews.apache.org/r/60400/diff/1/
Testing
---
Thanks,
Jiang Yan Xu
}
```
src/slave/slave.cpp
Line 6003 (original), 6007 (patched)
<https://reviews.apache.org/r/60105/#comment253017>
Empty line above.
src/slave/slave.cpp
Line 6004 (original), 6019 (patched)
<https://reviews.apache.org/r/60105/#comment253021>
Empty line
)
<https://reviews.apache.org/r/60104/#comment252967>
No need for this?
- Jiang Yan Xu
On June 22, 2017, 2:17 p.m., Megha Sharma wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://
ault here? It's more likely that the
> > segfault was caused by a dereferencing a non-`nullptr` process that had
> > already been deallocated (which would not be fixed by this review).
>
> Jiang Yan Xu wrote:
> The use case is something like this:
>
> ht
ize() so it segfaulted.
Just felt it's not really necessary to fail (and it used to not) in this way?
- Jiang Yan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60252/#review178522
----------
to date and fill out the
testing done section, even if it's (e.g.,) "make check together with /r/56895/".
- Jiang Yan Xu
On June 20, 2017, 8:41 a.m., Megha Sharma wrote:
>
> ---
> This is an automatically generated
agent, we are just **not** going to
execute `info = slaveState->info.get();` below?
src/slave/slave.cpp
Line 5965 (original)
<https://reviews.apache.org/r/60105/#comment252444>
Why remove it?
src/slave/slave.cpp
Lines 6122-6125 (original)
<https://reviews.apache.org/r/60105/#com
---
We don't need and shouldn't assume pointers in `processes` are
non-nullptr.
Diffs
-
3rdparty/libprocess/src/process.cpp 7ce6d2b13baa68906e091a95c0dd58ee1ca2bc7d
Diff: https://reviews.apache.org/r/60252/diff/1/
Testing
---
make check.
Thanks,
Jiang Yan Xu
nded
and document the commits with more context about what changed and why.
- Jiang Yan Xu
On June 15, 2017, 12:31 p.m., Megha Sharma wrote:
>
> ---
> This is an automatically generate
of the
variable.
If you search `Future _statusUpdateAcknowledgement =` in the same
file you can actually see plenty of examples. :)
- Jiang Yan Xu
On June 15, 2017, 12:30 p.m., Megha Sharma wrote:
>
> ---
> This is an
executor dir using this mode would
solve this problem?
- Jiang Yan Xu
On June 16, 2017, 4:45 p.m., Silas Snider 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/59841/#review177070
---
Ship it!
Ship It!
- Jiang Yan Xu
On June 6, 2017, 7:48 a.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59734/#review176704
---
Ship it!
Ship It!
- Jiang Yan Xu
On June 1, 2017, 3:35 p.m
rces.cpp
Line 619 (original), 619-624 (patched)
<https://reviews.apache.org/r/51879/#comment249923>
This is exatctly the content of `Resources::fromString` (which was added
after we introduced `Resources::fromSimpleString`).
- Jiang Yan Xu
On May 24, 2017, 12:56 p.m., Anindya Sinha wrote:
> On Dec. 15, 2016, 11:58 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/containerizer.cpp
> > Lines 165-169 (original), 288-305 (patched)
> > <https://reviews.apache.org/r/51879/diff/11/?file=1581274#file1581274line327>
> >
> >
automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59254/#review176348
-----------
On May 30, 2017, 11:12 a.m., Jiang Yan Xu wrote:
>
> ---
> This is a
> On May 24, 2017, 10:26 a.m., Jiang Yan Xu wrote:
> > Hmm, is this also violating the convention then?
> >
> > https://github.com/apache/mesos/blob/d225d4d4122e773e2416ba0d0eee653da8ced352/include/mesos/authorizer/acls.proto#L344
> >
> > What if later we use
make check.
Thanks,
Jiang Yan Xu
is invoked, how
much overhead would it cost the sort()? This is probably worth measuring if we
add these.
- Jiang Yan Xu
On May 23, 2017, 9:23 p.m., Anindya Sinha wrote:
>
> ---
> This is an automatically generated e-mail.
> On May 24, 2017, 10:26 a.m., Jiang Yan Xu wrote:
> > Hmm, is this also violating the convention then?
> >
> > https://github.com/apache/mesos/blob/d225d4d4122e773e2416ba0d0eee653da8ced352/include/mesos/authorizer/acls.proto#L344
> >
> > What if later we use
Yan Xu
On May 24, 2017, 1:12 a.m., Alexander Rojas wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59453/
> --
), 286 (patched)
<https://reviews.apache.org/r/59506/#comment249231>
Not yours but the trailing period is inconsistent with our convention.
Remove it?
src/slave/main.cpp
Line 293 (original), 291 (patched)
<https://reviews.apache.org/r/59506/#comment249232>
Ditto.
- Jiang
ing.type);
EXPECT_GE(timing.as(), 0.0);
// The statistics should be generated.
foreach (const string& statistic, statistics) {
EXPECT_EQ(1u, values.count(statistic))
<< "Expected " << statistic << " to
> On May 12, 2017, 10:13 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/utils.cpp
> > Lines 157-158 (original), 155-156 (patched)
> > <https://reviews.apache.org/r/55895/diff/6/?file=1713122#file1713122line157>
> >
> > Sorry
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55895/#review175768
---
Ship it!
Ship It!
- Jiang Yan Xu
On May 16, 2017, 4:09 p.m
> On May 12, 2017, 10:13 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/utils.cpp
> > Lines 157-158 (original), 155-156 (patched)
> > <https://reviews.apache.org/r/55895/diff/6/?file=1713122#file1713122line157>
> >
> > Sorry
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59434/#review175652
---
On May 21, 2017, 10:38 p.m., Jiang Yan Xu wrote:
>
>
---
- On the agent (re)-registration code paths.
- Could be helpful for triaging performance issues.
Diffs
-
src/master/master.cpp 02affe2d6dc76ef91363df04d8d8cbed3beaf34f
Diff: https://reviews.apache.org/r/59434/diff/1/
Testing
---
make check.
Thanks,
Jiang Yan Xu
167126ee694b
Diff: https://reviews.apache.org/r/59290/diff/2/
Changes: https://reviews.apache.org/r/59290/diff/1-2/
Testing
---
make check.
Thanks,
Jiang Yan Xu
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56681/#review175185
---
Ship it!
Ship It!
- Jiang Yan Xu
On May 16, 2017, 4:19 p.m
Description
---
Updated documentation for `registrar/log/network_size`.
Diffs
-
docs/monitoring.md c42afce0a26dc15e033ee1375b7cb23d55be40ab
Diff: https://reviews.apache.org/r/59300/diff/1/
Testing
---
Markdown viewer.
Thanks,
Jiang Yan Xu
cs.cpp PRE-CREATION
src/tests/log_tests.cpp 52b1bfedd8d3f8f2ab3308c4be9f167126ee694b
Diff: https://reviews.apache.org/r/59290/diff/1/
Testing
---
make check.
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/59194/#review175033
---
Ship it!
Ship It!
- Jiang Yan Xu
On May 15, 2017, 11:01 a.m
he code. The error message also
reminds the reader about the "additional shared copy". I feel it's clear enough
without the sentence.
src/tests/resources_tests.cpp
Lines 2577 (patched)
<https://reviews.apache.org/r/59194/#comment248202>
`__total`?
- Jiang Yan
-
>
> (Updated April 21, 2017, 10:53 a.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
>
>
> Repository: mesos
>
>
> Description
> ---
g the same for the new metric. Can we create an independent test?
src/tests/hierarchical_allocator_tests.cpp
Line 3544 (original), 3544 (patched)
<https://reviews.apache.org/r/53840/#comment248195>
Can it realistically be 0.0?
- Jiang Yan Xu
On April 21, 2017, 10:53 a
chical.hpp
123f97cf495bff0f822838e09df0d88818f04da6
src/master/allocator/mesos/hierarchical.cpp
b75ed9a20a9a42f958cebbacd91e5e15b0d21394
Diff: https://reviews.apache.org/r/59254/diff/1/
Testing
---
make check.
Thanks,
Jiang Yan Xu
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56681/#review174837
---
Could you update the "testing done" section?
- Jiang Y
Why are we testing CREATE? This test can just literally be a DESTROY
operation on a resources object with two volumes (which is simpler) right?
- Jiang Yan Xu
On May 11, 2017, 1:21 p.m., Anindya Sinha wrote:
>
> ---
> This is a
> On May 12, 2017, 10:13 a.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/utils.cpp
> > Lines 157-158 (original), 155-156 (patched)
> > <https://reviews.apache.org/r/55895/diff/6/?file=1713122#file1713122line157>
> >
> > Sorry
low
`limit == 1` if we allow `limit == 513`?
Since we don't need `BasicBlocks::BASIC_BLOCK_SIZE` here, we can make it
private.
Even in some hypothetical scenarios where we needed to compare bytes and
BasicBlocks, it should probably be `limit < BasicBlocks(1).bytes()` or we can
m
(patched)
<https://reviews.apache.org/r/55903/#comment247915>
s/only//
- Jiang Yan Xu
On May 11, 2017, 10:55 a.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/56680/#review174611
---
Ship it!
Ship It!
- Jiang Yan Xu
On May 8, 2017, 4:29 p.m
n
just paste this command to get what they want if they use the isolator.
- Jiang Yan Xu
On April 25, 2017, 3:38 p.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> http
he.org/r/55897/#comment247788>
Ditto.
- Jiang Yan Xu
On May 2, 2017, 10:23 a.m., James Peach wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59154/#review174530
---
Ship it!
Ship It!
- Jiang Yan Xu
On May 10, 2017, 11:59 a.m
ent247527>
We can group the two statements and remove the redundant "// A partial
block should round up."?
- Jiang Yan Xu
On April 29, 2017, 10:41 a.m., James Peach wrote:
>
> ---
> This is an automatically generated e
src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31
Diff: https://reviews.apache.org/r/58587/diff/2/
Changes: https://reviews.apache.org/r/58587/diff/1-2/
Testing
---
N/A
Thanks,
Jiang Yan Xu
don't get what "a
> > persistent volume can be destroyed if it is not allocated" means..
>
> Jiang Yan Xu wrote:
> Ok I guess I shoud say "a persistent volume can be destroyed only if it
> is not used". In this case, it's not going to be pendi
it:
> https://reviews.apache.org/r/56681/
> ---
>
> (Updated May 8, 2017, 4:30 p.m.)
>
>
> Review request for mesos, haosdent huang and Jiang Yan Xu.
>
>
> Bugs: MESOS-7115
> https://issues.apache.org/jira/browse/MESOS-7115
/persistent_volume_tests.cpp
Lines 1192 (patched)
<https://reviews.apache.org/r/57964/#comment246757>
s/populated/correctly populated/
- Jiang Yan Xu
On May 2, 2017, 10:54 a.m., Anindya Sinha wrote:
>
> ---
> This is an automatica
-
>
> (Updated May 2, 2017, 10:53 a.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-7186
> https://issues.apache.org/jira/browse/MESOS-7186
>
>
> Repository: mesos
>
>
> Description
> ---
>
> T
58892/diff/1-2/
Testing
---
Markdown viewer.
Thanks,
Jiang Yan Xu
s to which one
> > should be used?
+1.
- Jiang Yan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58892/#review173566
-----------
views.apache.org/r/58587/#review173561
---
On May 1, 2017, 2:29 p.m., Jiang Yan Xu wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://revie
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58916/#review173597
---
Ship it!
Ship It!
- Jiang Yan Xu
On May 2, 2017, 1:26 a.m
-
>
> (Updated April 22, 2017, 10 a.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-7186
> https://issues.apache.org/jira/browse/MESOS-7186
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Adde
We could use the same wording for the `slaveUsed` case above so all of
these comments are consistent (which was my original suggestion).
- Jiang Yan Xu
On April 22, 2017, 10 a.m., Anindya Sinha wrote:
>
> ---
> This i
e.org/r/58587/diff/1/
Testing
---
N/A
Thanks,
Jiang Yan Xu
---
We've been using it for new code now.
Diffs
-
docs/c++-style-guide.md ccb81f48e2edc9c1e7c328cc26e44d658b4c43b4
Diff: https://reviews.apache.org/r/58892/diff/1/
Testing
---
Markdown viewer.
Thanks,
Jiang Yan Xu
isk_used_bytes()` is even if I know it's below
`usage->has_disk_limit_bytes()`: it being zero vs. not may help me debug.
```
ASSERT_FALSE(timeout.expired())
<< "Disk usage failed to reach " << usage->has_disk_limit_bytes()
<< " when tim
Let's chat if you feel strongly about advocating for this style.
src/tests/containerizer/xfs_quota_tests.cpp
Lines 948 (patched)
<https://reviews.apache.org/r/55895/#comment246422>
Be consistent in the use of unsigned literal.
- Jiang Yan Xu
On April 25, 2017, 3:37
> On April 28, 2017, 4:06 p.m., Jiang Yan Xu wrote:
> > src/linux/perf.cpp
> > Line 322 (original), 313 (patched)
> > <https://reviews.apache.org/r/56732/diff/1/?file=1635867#file1635867line322>
> >
> > Now without the `process::collect`.
&
le: " + result.error());
}
foreachvalue (mesos::PerfStatistics& statistics, result.get()) {
statistics.set_timestamp(start.secs());
statistics.set_duration(duration.secs());
}
return result.get();
});
```
- Jiang Yan Xu
On Fe
://reviews.apache.org/r/57534/diff/9/
Changes: https://reviews.apache.org/r/57534/diff/8-9/
Testing
---
make check.
Thanks,
Jiang Yan Xu
://reviews.apache.org/r/57534/diff/8/
Changes: https://reviews.apache.org/r/57534/diff/7-8/
Testing
---
make check.
Thanks,
Jiang Yan Xu
/r/58676/diff/1/
Testing
---
make check.
Thanks,
Jiang Yan Xu
/7-8/
Testing
---
make check.
The tests added here cover some basic scenarios, I have more tests but will add
them when MESOS-7244 is fixed.
Thanks,
Jiang Yan Xu
> On April 23, 2017, 10:48 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/isolators/xfs/utils.hpp
> > Lines 65 (patched)
> > <https://reviews.apache.org/r/55897/diff/2/?file=1667425#file1667425line65>
> >
> > We started to move toward `
ment mode and don't set it at all in the accounting-only mode?
I could be totally mistaken but if I am, we should improve the
documentation to clarify for the readers.
- Jiang Yan Xu
On March 17, 2017, 2:57 p.m., James Peach wrote:
>
>
4>
"sleep 1000" above but "sleep 500" here? Keep them consistent ("sleep
1000")?
src/tests/persistent_volume_tests.cpp
Lines 1326-1327 (patched)
<https://reviews.apache.org/r/57964/#comment245769>
Actually this doesn't look necessary, we already wait for
> On April 21, 2017, 10:37 a.m., Jiang Yan Xu wrote:
> > LGTM!
> >
> > Can we have a test? It doesn't have to be a standalone test if we already
> > have a test that just launches tasks from two frameworks, then we can just
> > add it to the test? If not
ttps://reviews.apache.org/r/57963/#comment245734>
Ditto.
- Jiang Yan Xu
On March 27, 2017, 3:27 p.m., Anindya Sinha wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
)
-
src/tests/script.cpp 3b68b845b06fe19acb8b08e1ff3dd0bec9117f05
Diff: https://reviews.apache.org/r/57730/diff/2/
Changes: https://reviews.apache.org/r/57730/diff/1-2/
Testing
---
make check
Thanks,
Jiang Yan Xu
sts, so I'd
> > expect the agents to try to register without authentication. Should we
> > enable agent authentication (and authorization) by default in the tests,
> > and only disable it for the rare few tests that don't need it?
>
> Jiang Yan Xu wrote:
> G
/57535/diff/6-7/
Testing
---
make check.
The tests added here cover some basic scenarios, I have more tests but will add
them when MESOS-7244 is fixed.
Thanks,
Jiang Yan Xu
`newAllocation` have the same quantity and then do not modify
`allocations[name].scalarQuantities` or
`allocations[name].totals[resource.name()]` at all. This is basically what the
other does?
- Jiang Yan Xu
On April 18, 2017, 7:50 p.m., Ani
ered out
due to being removed from `slave.total` in the allocator, the allocator is
itself running and the framework's incorrect allocation can cause issue, e.g.,
fairness, although there are probably more severe problems.
- Jiang Yan Xu
On April 18, 2017, 7:50 p.m., Anindya Sinha wrote:
>
eviews.apache.org/r/58485/#comment245379>
s/offers/offer/?
- Jiang Yan Xu
On April 17, 2017, 4:14 p.m., Anindya Sinha 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/57730/#review171536
-----------
On March 17, 2017, 8:53 a.m., Jiang Yan Xu wrote:
>
> ---
until we determine their fate.
hashmap recovered;
```
- Jiang Yan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57535/#review170056
-
-6/
Testing
---
make check.
The tests added here cover some basic scenarios, I have more tests but will add
them when MESOS-7244 is fixed.
Thanks,
Jiang Yan Xu
://reviews.apache.org/r/57534/diff/6/
Changes: https://reviews.apache.org/r/57534/diff/5-6/
Testing
---
make check.
Thanks,
Jiang Yan Xu
rces.nonRevocable();
}
used +=
slaveUsed.get(name).getOrElse(Value::Scalar()).value();
}
return used;
}
```
Here and below.
- Jiang Yan Xu
On March 27, 2017, 11:26 a.m., Anindya Sinha wrote:
>
> ---
://reviews.apache.org/r/57535/diff/5/
Changes: https://reviews.apache.org/r/57535/diff/4-5/
Testing
---
make check.
The tests added here cover some basic scenarios, I have more tests but will add
them when MESOS-7244 is fixed.
Thanks,
Jiang Yan Xu
cy does exist for this case
elsewhere but to make this method more consistency let's follow the general
principle of not printing backtrace in cases where failures are due to external
conditions.
src/slave/slave.cpp
Lines 5546-5552 (original), 5570-5577 (patched)
<https://r
ing in the agent's reregistration code path to verify?
Good catch. I created a `slaveFlags` above but forgot to use it. I should've
used `FUTURE_MESSAGE(Eq(SlaveReregisteredMessage().GetTypeName()), _, _);` and
that should confirm it.
- Jiang Yan
----------
added here cover some basic scenarios, I have more tests but will add
them when MESOS-7244 is fixed.
Thanks,
Jiang Yan Xu
://reviews.apache.org/r/57534/diff/4/
Changes: https://reviews.apache.org/r/57534/diff/3-4/
Testing
---
make check.
Thanks,
Jiang Yan Xu
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57534/#review169323
-----------
On March 14, 2017, 5:40 p.m., Jiang Yan Xu wrote:
>
> --
ivate:
BoundedHashMap frameworkIds;
BoundedHashMap pids;
BoundedHashMap> streamIds;
} completed;
} frameworks;
```
src/master/master.cpp
Lines 2654 (patched)
<https://reviews.apache.org/r/57739/#comment241730>
Keep the error message consistent: "F
--
>
> (Updated March 16, 2017, 11:25 a.m.)
>
>
> Review request for mesos, Neil Conway and Jiang Yan Xu.
>
>
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
>
>
> Repository: mesos
>
>
> Description
> ---
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57712/#review169259
---
Ship it!
- Jiang Yan Xu
On March 16, 2017, 5 p.m., Anindya
true.
Diffs
-
src/tests/master_authorization_tests.cpp
1a0285a3f345ef21a8256d4123d8bb684f538da4
Diff: https://reviews.apache.org/r/57731/diff/1/
Testing
---
make check.
Thanks,
Jiang Yan Xu
: https://reviews.apache.org/r/57730/diff/1/
Testing
---
make check
Thanks,
Jiang Yan Xu
://reviews.apache.org/r/57710/diff/1/
Testing (updated)
---
via a markdown viewer.
Thanks,
Jiang Yan Xu
201 - 300 of 1075 matches
Mail list logo
