You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2017/01/23 19:08:39 UTC
Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/
-----------------------------------------------------------
Review request for mesos, Guangya Liu and Jacob Janco.
Bugs: MESOS-6904
https://issues.apache.org/jira/browse/MESOS-6904
Repository: mesos
Description
-------
- This test is broken by the batched allocation change.
Diffs
-----
src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
Diff: https://reviews.apache.org/r/55852/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jacob Janco <jj...@gmail.com>.
> On Jan. 30, 2017, 11:51 p.m., Jacob Janco wrote:
> > Ship It!
Makes sense, with batched allocation recovery and allocation can be interleaved causing flakiness in this test.
- Jacob
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163594
-----------------------------------------------------------
On Jan. 23, 2017, 7:08 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2017, 7:08 p.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jacob Janco <jj...@gmail.com>.
> On Jan. 30, 2017, 11:51 p.m., Jacob Janco wrote:
> > Ship It!
>
> Jacob Janco wrote:
> Makes sense, with batched allocation recovery and allocation can be interleaved causing flakiness in this test.
Passed --gtest_repeat=1000
- Jacob
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163594
-----------------------------------------------------------
On Jan. 23, 2017, 7:08 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2017, 7:08 p.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jacob Janco <jj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163594
-----------------------------------------------------------
Ship it!
Ship It!
- Jacob Janco
On Jan. 23, 2017, 7:08 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2017, 7:08 p.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review162732
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55852]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Jan. 23, 2017, 7:08 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2017, 7:08 p.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Guangya Liu <gy...@gmail.com>.
> On \u4e8c\u6708 1, 2017, 1:06 a.m., Guangya Liu wrote:
> > src/tests/master_allocator_tests.cpp, lines 1751-1752
> > <https://reviews.apache.org/r/55852/diff/1/?file=1612985#file1612985line1751>
> >
> > This is only checking one offer but not two?
>
> Jiang Yan Xu wrote:
> `framework2offers` is a `Queue`, calling [get()](https://github.com/apache/mesos/blob/d0f4cd4d9e98c27fd03eeafcef6e16bb04f51bc8/3rdparty/libprocess/include/process/queue.hpp#L55) on it repeatedly will pop out the elements.
>
> Guangya Liu wrote:
> Ah, I see, can you please update the comments a bit here to reflect this, then we can ship it.
>
> Jiang Yan Xu wrote:
> What would you like to see? How about
>
> ```
> // All offers for framework2 are enqueued by now.
> ```
>
> ? Or did you mean a comment about `Queue::get()`?
>
> The latter should probably be a comment in `Queue` and not here right?
>
> Guangya Liu wrote:
> How about
>
> ```
> // All offers for framework2 are enqueued by now, the
> // `Queue::get()` will pop out the elements.
> ```
>
> BTW: I did not saw any comments in `Queue::get()`, so it would be great if we can put some comments here, perhaps also need a follow up patch to update the comments for `Queue::get()`, comments?
@Yan, just think more, how about update the function name for `Queue` from `Queue::put` and `Queue::get` to `Queue::push` and `Queue::pop`? My thinking is `get` should be a `readonly` operation.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
On \u4e00\u6708 31, 2017, 9:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 31, 2017, 9:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jiang Yan Xu <xu...@apple.com>.
> On Jan. 31, 2017, 5:06 p.m., Guangya Liu wrote:
> > src/tests/master_allocator_tests.cpp, lines 1751-1752
> > <https://reviews.apache.org/r/55852/diff/1/?file=1612985#file1612985line1751>
> >
> > This is only checking one offer but not two?
>
> Jiang Yan Xu wrote:
> `framework2offers` is a `Queue`, calling [get()](https://github.com/apache/mesos/blob/d0f4cd4d9e98c27fd03eeafcef6e16bb04f51bc8/3rdparty/libprocess/include/process/queue.hpp#L55) on it repeatedly will pop out the elements.
>
> Guangya Liu wrote:
> Ah, I see, can you please update the comments a bit here to reflect this, then we can ship it.
What would you like to see? How about
```
// All offers for framework2 are enqueued by now.
```
? Or did you mean a comment about `Queue::get()`?
The latter should probably be a comment in `Queue` and not here right?
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
On Jan. 31, 2017, 1:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 1:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Guangya Liu <gy...@gmail.com>.
> On \u4e8c\u6708 1, 2017, 1:06 a.m., Guangya Liu wrote:
> > src/tests/master_allocator_tests.cpp, lines 1751-1752
> > <https://reviews.apache.org/r/55852/diff/1/?file=1612985#file1612985line1751>
> >
> > This is only checking one offer but not two?
>
> Jiang Yan Xu wrote:
> `framework2offers` is a `Queue`, calling [get()](https://github.com/apache/mesos/blob/d0f4cd4d9e98c27fd03eeafcef6e16bb04f51bc8/3rdparty/libprocess/include/process/queue.hpp#L55) on it repeatedly will pop out the elements.
Ah, I see, can you please update the comments a bit here to reflect this, then we can ship it.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
On \u4e00\u6708 31, 2017, 9:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 31, 2017, 9:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jiang Yan Xu <xu...@apple.com>.
> On Jan. 31, 2017, 5:06 p.m., Guangya Liu wrote:
> > src/tests/master_allocator_tests.cpp, lines 1751-1752
> > <https://reviews.apache.org/r/55852/diff/1/?file=1612985#file1612985line1751>
> >
> > This is only checking one offer but not two?
`framework2offers` is a `Queue`, calling [get()](https://github.com/apache/mesos/blob/d0f4cd4d9e98c27fd03eeafcef6e16bb04f51bc8/3rdparty/libprocess/include/process/queue.hpp#L55) on it repeatedly will pop out the elements.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
On Jan. 31, 2017, 1:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 1:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Guangya Liu <gy...@gmail.com>.
> On \u4e8c\u6708 1, 2017, 1:06 a.m., Guangya Liu wrote:
> > src/tests/master_allocator_tests.cpp, lines 1751-1752
> > <https://reviews.apache.org/r/55852/diff/1/?file=1612985#file1612985line1751>
> >
> > This is only checking one offer but not two?
>
> Jiang Yan Xu wrote:
> `framework2offers` is a `Queue`, calling [get()](https://github.com/apache/mesos/blob/d0f4cd4d9e98c27fd03eeafcef6e16bb04f51bc8/3rdparty/libprocess/include/process/queue.hpp#L55) on it repeatedly will pop out the elements.
>
> Guangya Liu wrote:
> Ah, I see, can you please update the comments a bit here to reflect this, then we can ship it.
>
> Jiang Yan Xu wrote:
> What would you like to see? How about
>
> ```
> // All offers for framework2 are enqueued by now.
> ```
>
> ? Or did you mean a comment about `Queue::get()`?
>
> The latter should probably be a comment in `Queue` and not here right?
>
> Guangya Liu wrote:
> How about
>
> ```
> // All offers for framework2 are enqueued by now, the
> // `Queue::get()` will pop out the elements.
> ```
>
> BTW: I did not saw any comments in `Queue::get()`, so it would be great if we can put some comments here, perhaps also need a follow up patch to update the comments for `Queue::get()`, comments?
>
> Guangya Liu wrote:
> @Yan, just think more, how about update the function name for `Queue` from `Queue::put` and `Queue::get` to `Queue::push` and `Queue::pop`? My thinking is `get` should be a `readonly` operation.
>
> Jiang Yan Xu wrote:
> I am reluctant to add comments to explain a particular API. We can certainly improve docuemntation around `Queue`, would you like to file a JIRA?
>
> As for there, how about I make the code more obvious?
>
> ```
> for (int i = 0; i < 2; i++) {
> Future<Offer> offer = framework2offers.get();
>
> // All offers for framework2 are enqueued by now.
> AWAIT_READY(offer);
> EXPECT_EQ(Resources(offer->resources()),
> Resources::parse(agentResources).get());
> }
> ```
>
> Jiang Yan Xu wrote:
> re push/pop, it's a separate discussion but my feeling is that `pop()` doesn't convey the sense that it's an async operation but agreed that perhaps there could be a better name than `get()`.
re update code, yes, it is better to make the code obvious, also file JIRA https://issues.apache.org/jira/browse/MESOS-7044 to trace this.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
On \u4e00\u6708 31, 2017, 9:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 31, 2017, 9:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jiang Yan Xu <xu...@apple.com>.
> On Jan. 31, 2017, 5:06 p.m., Guangya Liu wrote:
> > src/tests/master_allocator_tests.cpp, lines 1751-1752
> > <https://reviews.apache.org/r/55852/diff/1/?file=1612985#file1612985line1751>
> >
> > This is only checking one offer but not two?
>
> Jiang Yan Xu wrote:
> `framework2offers` is a `Queue`, calling [get()](https://github.com/apache/mesos/blob/d0f4cd4d9e98c27fd03eeafcef6e16bb04f51bc8/3rdparty/libprocess/include/process/queue.hpp#L55) on it repeatedly will pop out the elements.
>
> Guangya Liu wrote:
> Ah, I see, can you please update the comments a bit here to reflect this, then we can ship it.
>
> Jiang Yan Xu wrote:
> What would you like to see? How about
>
> ```
> // All offers for framework2 are enqueued by now.
> ```
>
> ? Or did you mean a comment about `Queue::get()`?
>
> The latter should probably be a comment in `Queue` and not here right?
>
> Guangya Liu wrote:
> How about
>
> ```
> // All offers for framework2 are enqueued by now, the
> // `Queue::get()` will pop out the elements.
> ```
>
> BTW: I did not saw any comments in `Queue::get()`, so it would be great if we can put some comments here, perhaps also need a follow up patch to update the comments for `Queue::get()`, comments?
>
> Guangya Liu wrote:
> @Yan, just think more, how about update the function name for `Queue` from `Queue::put` and `Queue::get` to `Queue::push` and `Queue::pop`? My thinking is `get` should be a `readonly` operation.
>
> Jiang Yan Xu wrote:
> I am reluctant to add comments to explain a particular API. We can certainly improve docuemntation around `Queue`, would you like to file a JIRA?
>
> As for there, how about I make the code more obvious?
>
> ```
> for (int i = 0; i < 2; i++) {
> Future<Offer> offer = framework2offers.get();
>
> // All offers for framework2 are enqueued by now.
> AWAIT_READY(offer);
> EXPECT_EQ(Resources(offer->resources()),
> Resources::parse(agentResources).get());
> }
> ```
re push/pop, it's a separate discussion but my feeling is that `pop()` doesn't convey the sense that it's an async operation but agreed that perhaps there could be a better name than `get()`.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
On Jan. 31, 2017, 1:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 1:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jiang Yan Xu <xu...@apple.com>.
> On Jan. 31, 2017, 5:06 p.m., Guangya Liu wrote:
> > src/tests/master_allocator_tests.cpp, lines 1751-1752
> > <https://reviews.apache.org/r/55852/diff/1/?file=1612985#file1612985line1751>
> >
> > This is only checking one offer but not two?
>
> Jiang Yan Xu wrote:
> `framework2offers` is a `Queue`, calling [get()](https://github.com/apache/mesos/blob/d0f4cd4d9e98c27fd03eeafcef6e16bb04f51bc8/3rdparty/libprocess/include/process/queue.hpp#L55) on it repeatedly will pop out the elements.
>
> Guangya Liu wrote:
> Ah, I see, can you please update the comments a bit here to reflect this, then we can ship it.
>
> Jiang Yan Xu wrote:
> What would you like to see? How about
>
> ```
> // All offers for framework2 are enqueued by now.
> ```
>
> ? Or did you mean a comment about `Queue::get()`?
>
> The latter should probably be a comment in `Queue` and not here right?
>
> Guangya Liu wrote:
> How about
>
> ```
> // All offers for framework2 are enqueued by now, the
> // `Queue::get()` will pop out the elements.
> ```
>
> BTW: I did not saw any comments in `Queue::get()`, so it would be great if we can put some comments here, perhaps also need a follow up patch to update the comments for `Queue::get()`, comments?
>
> Guangya Liu wrote:
> @Yan, just think more, how about update the function name for `Queue` from `Queue::put` and `Queue::get` to `Queue::push` and `Queue::pop`? My thinking is `get` should be a `readonly` operation.
I am reluctant to add comments to explain a particular API. We can certainly improve docuemntation around `Queue`, would you like to file a JIRA?
As for there, how about I make the code more obvious?
```
for (int i = 0; i < 2; i++) {
Future<Offer> offer = framework2offers.get();
// All offers for framework2 are enqueued by now.
AWAIT_READY(offer);
EXPECT_EQ(Resources(offer->resources()),
Resources::parse(agentResources).get());
}
```
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
On Jan. 31, 2017, 1:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 1:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Guangya Liu <gy...@gmail.com>.
> On \u4e8c\u6708 1, 2017, 1:06 a.m., Guangya Liu wrote:
> > src/tests/master_allocator_tests.cpp, lines 1751-1752
> > <https://reviews.apache.org/r/55852/diff/1/?file=1612985#file1612985line1751>
> >
> > This is only checking one offer but not two?
>
> Jiang Yan Xu wrote:
> `framework2offers` is a `Queue`, calling [get()](https://github.com/apache/mesos/blob/d0f4cd4d9e98c27fd03eeafcef6e16bb04f51bc8/3rdparty/libprocess/include/process/queue.hpp#L55) on it repeatedly will pop out the elements.
>
> Guangya Liu wrote:
> Ah, I see, can you please update the comments a bit here to reflect this, then we can ship it.
>
> Jiang Yan Xu wrote:
> What would you like to see? How about
>
> ```
> // All offers for framework2 are enqueued by now.
> ```
>
> ? Or did you mean a comment about `Queue::get()`?
>
> The latter should probably be a comment in `Queue` and not here right?
How about
```
// All offers for framework2 are enqueued by now, the
// `Queue::get()` will pop out the elements.
```
BTW: I did not saw any comments in `Queue::get()`, so it would be great if we can put some comments here, perhaps also need a follow up patch to update the comments for `Queue::get()`, comments?
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
On \u4e00\u6708 31, 2017, 9:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 31, 2017, 9:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/#review163762
-----------------------------------------------------------
src/tests/master_allocator_tests.cpp (lines 1750 - 1751)
<https://reviews.apache.org/r/55852/#comment235266>
This is only checking one offer but not two?
- Guangya Liu
On \u4e00\u6708 31, 2017, 9:03 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55852/
> -----------------------------------------------------------
>
> (Updated \u4e00\u6708 31, 2017, 9:03 a.m.)
>
>
> Review request for mesos, Guangya Liu and Jacob Janco.
>
>
> Bugs: MESOS-6904
> https://issues.apache.org/jira/browse/MESOS-6904
>
>
> Repository: mesos
>
>
> Description
> -------
>
> - This test is broken by the batched allocation change.
>
>
> Diffs
> -----
>
> src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
>
> Diff: https://reviews.apache.org/r/55852/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jiang Yan Xu <xu...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/
-----------------------------------------------------------
(Updated Feb. 1, 2017, 11:19 a.m.)
Review request for mesos, Guangya Liu and Jacob Janco.
Changes
-------
Comments. NNFR.
Bugs: MESOS-6904
https://issues.apache.org/jira/browse/MESOS-6904
Repository: mesos
Description
-------
- This test is broken by the batched allocation change.
Diffs (updated)
-----
src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
Diff: https://reviews.apache.org/r/55852/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 55852: Fixed
MasterAllocatorTest/1.RebalancedForUpdatedWeights.
Posted by Jiang Yan Xu <xu...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55852/
-----------------------------------------------------------
(Updated Jan. 31, 2017, 1:03 a.m.)
Review request for mesos, Guangya Liu and Jacob Janco.
Bugs: MESOS-6904
https://issues.apache.org/jira/browse/MESOS-6904
Repository: mesos
Description
-------
- This test is broken by the batched allocation change.
Diffs
-----
src/tests/master_allocator_tests.cpp 996762f25453f7a8a5e0b7b97006ee2a603cf8c4
Diff: https://reviews.apache.org/r/55852/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu