You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Ilya Pronin <ip...@twopensource.com> on 2017/09/12 12:59:54 UTC
Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/
-----------------------------------------------------------
Review request for mesos and Anand Mazumdar.
Bugs: MESOS-7867
https://issues.apache.org/jira/browse/MESOS-7867
Repository: mesos
Description
-------
This test verifies that we are able to upgrade from a PID based
framework to an HTTP framework and then downgrade back without
restarting the master.
Diffs
-----
src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
Diff: https://reviews.apache.org/r/62241/diff/1/
Testing
-------
Ran `make check`. Verified that the test fails without the previous patch.
Thanks,
Ilya Pronin
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/#review188204
-----------------------------------------------------------
PASS: Mesos patch 62241 was successfully built and tested.
Reviews applied: `['62240', '62241']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241
- Mesos Reviewbot Windows
On Oct. 16, 2017, 4:05 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 4:05 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
>
>
> Diffs
> -----
>
> src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
>
>
> Diff: https://reviews.apache.org/r/62241/diff/3/
>
>
> Testing
> -------
>
> Ran `make check`. Verified that the test fails without the previous patch.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/#review188203
-----------------------------------------------------------
Patch looks great!
Reviews applied: [62240, 62241]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Oct. 16, 2017, 8:05 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 8:05 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
>
>
> Diffs
> -----
>
> src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
>
>
> Diff: https://reviews.apache.org/r/62241/diff/3/
>
>
> Testing
> -------
>
> Ran `make check`. Verified that the test fails without the previous patch.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Ilya Pronin <ip...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/
-----------------------------------------------------------
(Updated Oct. 16, 2017, 1:05 p.m.)
Review request for mesos and Anand Mazumdar.
Changes
-------
Changed the comment.
Bugs: MESOS-7867
https://issues.apache.org/jira/browse/MESOS-7867
Repository: mesos
Description
-------
This test verifies that we are able to upgrade from a PID based
framework to an HTTP framework and then downgrade back without
restarting the master.
Diffs (updated)
-----
src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
Diff: https://reviews.apache.org/r/62241/diff/3/
Changes: https://reviews.apache.org/r/62241/diff/2-3/
Testing
-------
Ran `make check`. Verified that the test fails without the previous patch.
Thanks,
Ilya Pronin
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Ilya Pronin <ip...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/
-----------------------------------------------------------
(Updated Oct. 16, 2017, 11:17 a.m.)
Review request for mesos and Anand Mazumdar.
Changes
-------
Addressed review comments.
Bugs: MESOS-7867
https://issues.apache.org/jira/browse/MESOS-7867
Repository: mesos
Description
-------
This test verifies that we are able to upgrade from a PID based
framework to an HTTP framework and then downgrade back without
restarting the master.
Diffs (updated)
-----
src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
Diff: https://reviews.apache.org/r/62241/diff/2/
Changes: https://reviews.apache.org/r/62241/diff/1-2/
Testing
-------
Ran `make check`. Verified that the test fails without the previous patch.
Thanks,
Ilya Pronin
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Ilya Pronin <ip...@twopensource.com>.
> On Sept. 12, 2017, 3:16 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Mesos failed to build.
> >
> > Reviews applied: [62240, 62241]
> >
> > All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241
> >
> > Relevant logs:
> >
> > - mesos-cmake-build.log:
> >
> > ??- - T h e C c o m p i l e r i d e n t i f i c a t i o n i s M S V C 1 9 . 1 0 . 2 5 0 1 9 . 0
> > - - T h e C X X c o m p i l e r i d e n t i f i c a t i o n i s M S V C 1 9 . 1 0 . 2 5 0 1 9 . 0
> > - - C h e c k f o r w o r k i n g C c o m p i l e r : C : / P r o g r a m F i l e s ( x 8 6 ) / M i c r o s o f t V i s u a l S t u d i o / 2 0 1 7 / C o m m u n i t y / V C / T o o l s / M S V C / 1 4 . 1 0 . 2 5 0 1 7 / b i n / H o s t X 6 4 / x 6 4 / c l . e x e
> > - - C h e c k f o r w o r k i n g C c o m p i l e r : C : / P r o g r a m F i l e s ( x 8 6 ) / M i c r o s o f t V i s u a l S t u d i o / 2 0 1 7 / C o m m u n i t y / V C / T o o l s / M S V C / 1 4 . 1 0 . 2 5 0 1 7 / b i n / H o s t X 6 4 / x 6 4 / c l . e x e - - w o r k s
> > - - D e t e c t i n g C c o m p i l e r A B I i n f o
> > - - D e t e c t i n g C c o m p i l e r A B I i n f o - d o n e
> > - - C h e c k f o r w o r k i n g C X X c o m p i l e r : C : / P r o g r a m F i l e s ( x 8 6 ) / M i c r o s o f t V i s u a l S t u d i o / 2 0 1 7 / C o m m u n i t y / V C / T o o l s / M S V C / 1 4 . 1 0 . 2 5 0 1 7 / b i n / H o s t X 6 4 / x 6 4 / c l . e x e
> > - - C h e c k f o r w o r k i n g C X X c o m p i l e r : C : / P r o g r a m F i l e s ( x 8 6 ) / M i c r o s o f t V i s u a l S t u d i o / 2 0 1 7 / C o m m u n i t y / V C / T o o l s / M S V C / 1 4 . 1 0 . 2 5 0 1 7 / b i n / H o s t X 6 4 / x 6 4 / c l . e x e - - w o r k s
> > - - D e t e c t i n g C X X c o m p i l e r A B I i n f o
> > - - D e t e c t i n g C X X c o m p i l e r A B I i n f o - d o n e
> > - - D e t e c t i n g C X X c o m p i l e f e a t u r e s
> > - - D e t e c t i n g C X X c o m p i l e f e a t u r e s - d o n e
> > - - c o t i r e 1 . 7 . 1 0 l o a d e d .
> > - - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> > - - * * * * * * * * * B e g i n n i n g M e s o s C M a k e c o n f i g u r a t i o n s t e p * * * * * * * * *
> > - - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> > - - I N S T A L L A T I O N P R E F I X : C : / P r o g r a m F i l e s / M e s o s
> > - - M A C H I N E S P E C S :
> > - - H o s t n a m e :
> > - - O S : W I N D O W S ( 1 0 . 0 . 1 4 3 9 3 )
> > - - A r c h : A M D 6 4
> > - - B i t M o d e :
> > - - B u i l d I D :
> > - - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> >
> > Full log available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241/logs/mesos-cmake-build.log
Build logs are weird:
```
Building Mesos
-- The C compiler identification is MSVC 19.10.25019.0
-- The CXX compiler identification is MSVC 19.10.25019.0
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2017/Community/VC/Tools/MSVC/14.10.25017/bin/HostX64/x64/cl.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- cotire 1.7.10 loaded.
-- ************************************************************
-- ********* Beginning Mesos CMake configuration step *********
-- ************************************************************
-- INSTALLATION PREFIX: C:/Program Files/Mesos
-- MACHINE SPECS:
-- Hostname:
-- OS: WINDOWS(10.0.14393)
-- Arch: AMD64
-- BitMode:
-- BuildID:
-- ************************************************************
Mesos failed to build.
```
- Ilya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/#review185183
-----------------------------------------------------------
On Sept. 12, 2017, 1:59 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
>
> (Updated Sept. 12, 2017, 1:59 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
>
>
> Diffs
> -----
>
> src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
>
>
> Diff: https://reviews.apache.org/r/62241/diff/1/
>
>
> Testing
> -------
>
> Ran `make check`. Verified that the test fails without the previous patch.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/#review185183
-----------------------------------------------------------
FAIL: Mesos failed to build.
Reviews applied: [62240, 62241]
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241
Relevant logs:
- mesos-cmake-build.log:
��- - T h e C c o m p i l e r i d e n t i f i c a t i o n i s M S V C 1 9 . 1 0 . 2 5 0 1 9 . 0
- - T h e C X X c o m p i l e r i d e n t i f i c a t i o n i s M S V C 1 9 . 1 0 . 2 5 0 1 9 . 0
- - C h e c k f o r w o r k i n g C c o m p i l e r : C : / P r o g r a m F i l e s ( x 8 6 ) / M i c r o s o f t V i s u a l S t u d i o / 2 0 1 7 / C o m m u n i t y / V C / T o o l s / M S V C / 1 4 . 1 0 . 2 5 0 1 7 / b i n / H o s t X 6 4 / x 6 4 / c l . e x e
- - C h e c k f o r w o r k i n g C c o m p i l e r : C : / P r o g r a m F i l e s ( x 8 6 ) / M i c r o s o f t V i s u a l S t u d i o / 2 0 1 7 / C o m m u n i t y / V C / T o o l s / M S V C / 1 4 . 1 0 . 2 5 0 1 7 / b i n / H o s t X 6 4 / x 6 4 / c l . e x e - - w o r k s
- - D e t e c t i n g C c o m p i l e r A B I i n f o
- - D e t e c t i n g C c o m p i l e r A B I i n f o - d o n e
- - C h e c k f o r w o r k i n g C X X c o m p i l e r : C : / P r o g r a m F i l e s ( x 8 6 ) / M i c r o s o f t V i s u a l S t u d i o / 2 0 1 7 / C o m m u n i t y / V C / T o o l s / M S V C / 1 4 . 1 0 . 2 5 0 1 7 / b i n / H o s t X 6 4 / x 6 4 / c l . e x e
- - C h e c k f o r w o r k i n g C X X c o m p i l e r : C : / P r o g r a m F i l e s ( x 8 6 ) / M i c r o s o f t V i s u a l S t u d i o / 2 0 1 7 / C o m m u n i t y / V C / T o o l s / M S V C / 1 4 . 1 0 . 2 5 0 1 7 / b i n / H o s t X 6 4 / x 6 4 / c l . e x e - - w o r k s
- - D e t e c t i n g C X X c o m p i l e r A B I i n f o
- - D e t e c t i n g C X X c o m p i l e r A B I i n f o - d o n e
- - D e t e c t i n g C X X c o m p i l e f e a t u r e s
- - D e t e c t i n g C X X c o m p i l e f e a t u r e s - d o n e
- - c o t i r e 1 . 7 . 1 0 l o a d e d .
- - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
- - * * * * * * * * * B e g i n n i n g M e s o s C M a k e c o n f i g u r a t i o n s t e p * * * * * * * * *
- - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
- - I N S T A L L A T I O N P R E F I X : C : / P r o g r a m F i l e s / M e s o s
- - M A C H I N E S P E C S :
- - H o s t n a m e :
- - O S : W I N D O W S ( 1 0 . 0 . 1 4 3 9 3 )
- - A r c h : A M D 6 4
- - B i t M o d e :
- - B u i l d I D :
- - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
Full log available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62241/logs/mesos-cmake-build.log
- Mesos Reviewbot Windows
On Sept. 12, 2017, 12:59 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
>
> (Updated Sept. 12, 2017, 12:59 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
>
>
> Diffs
> -----
>
> src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
>
>
> Diff: https://reviews.apache.org/r/62241/diff/1/
>
>
> Testing
> -------
>
> Ran `make check`. Verified that the test fails without the previous patch.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Ilya Pronin <ip...@twopensource.com>.
> On Oct. 13, 2017, 6:40 p.m., Anand Mazumdar wrote:
> > src/tests/scheduler_http_api_tests.cpp
> > Lines 737-747 (patched)
> > <https://reviews.apache.org/r/62241/diff/1/?file=1820104#file1820104line737>
> >
> > Kill this? We don't need this to test if the downgrade worked.
>
> Ilya Pronin wrote:
> The crash that this test reproduces occurs when the master handles a `Message` from the scheduler. We can remove the reconciliation part and the crash would still happen when the scheduler driver sends a `TEARDOWN` call message from `SchedulerProcess::stop()`. But I think it is better to be explicit here and not leave exercising the code that we were fixing to the cleanup logic.
>
> Anand Mazumdar wrote:
> I see. Can you modify the comment to also mention why are you exercising the message handling i.e., it used to crash the master previously? Otherwise, its not immediately obvious as to why it's being done here.
Done!
- Ilya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/#review188053
-----------------------------------------------------------
On Oct. 16, 2017, 1:05 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 1:05 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
>
>
> Diffs
> -----
>
> src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
>
>
> Diff: https://reviews.apache.org/r/62241/diff/3/
>
>
> Testing
> -------
>
> Ran `make check`. Verified that the test fails without the previous patch.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Ilya Pronin <ip...@twopensource.com>.
> On Oct. 13, 2017, 6:40 p.m., Anand Mazumdar wrote:
> > src/tests/scheduler_http_api_tests.cpp
> > Lines 674 (patched)
> > <https://reviews.apache.org/r/62241/diff/1/?file=1820104#file1820104line674>
> >
> > s/an/a
I'm pretty sure it's "an HTTP" :) https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol
> On Oct. 13, 2017, 6:40 p.m., Anand Mazumdar wrote:
> > src/tests/scheduler_http_api_tests.cpp
> > Lines 712-716 (patched)
> > <https://reviews.apache.org/r/62241/diff/1/?file=1820104#file1820104line712>
> >
> > Do you care about a `HEARTBEAT` even for this test?
No. I copied that part from the other test :) I think we can can remove this part and failover right after we get a `SUBSCRIBED` event. Ran the updated test with `--gtest_repeat=1000 --gtest_break_on_failure` to verify that I don't introduce any flakiness and it seems fine.
> On Oct. 13, 2017, 6:40 p.m., Anand Mazumdar wrote:
> > src/tests/scheduler_http_api_tests.cpp
> > Lines 737-747 (patched)
> > <https://reviews.apache.org/r/62241/diff/1/?file=1820104#file1820104line737>
> >
> > Kill this? We don't need this to test if the downgrade worked.
The crash that this test reproduces occurs when the master handles a `Message` from the scheduler. We can remove the reconciliation part and the crash would still happen when the scheduler driver sends a `TEARDOWN` call message from `SchedulerProcess::stop()`. But I think it is better to be explicit here and not leave exercising the code that we were fixing to the cleanup logic.
- Ilya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/#review188053
-----------------------------------------------------------
On Oct. 16, 2017, 11:17 a.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 11:17 a.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
>
>
> Diffs
> -----
>
> src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
>
>
> Diff: https://reviews.apache.org/r/62241/diff/2/
>
>
> Testing
> -------
>
> Ran `make check`. Verified that the test fails without the previous patch.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Anand Mazumdar <an...@apache.org>.
> On Oct. 14, 2017, 1:40 a.m., Anand Mazumdar wrote:
> > src/tests/scheduler_http_api_tests.cpp
> > Lines 737-747 (patched)
> > <https://reviews.apache.org/r/62241/diff/1/?file=1820104#file1820104line737>
> >
> > Kill this? We don't need this to test if the downgrade worked.
>
> Ilya Pronin wrote:
> The crash that this test reproduces occurs when the master handles a `Message` from the scheduler. We can remove the reconciliation part and the crash would still happen when the scheduler driver sends a `TEARDOWN` call message from `SchedulerProcess::stop()`. But I think it is better to be explicit here and not leave exercising the code that we were fixing to the cleanup logic.
I see. Can you modify the comment to also mention why are you exercising the message handling i.e., it used to crash the master previously? Otherwise, its not immediately obvious as to why it's being done here.
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/#review188053
-----------------------------------------------------------
On Oct. 16, 2017, 6:17 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:17 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
>
>
> Diffs
> -----
>
> src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
>
>
> Diff: https://reviews.apache.org/r/62241/diff/2/
>
>
> Testing
> -------
>
> Ran `make check`. Verified that the test fails without the previous patch.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 62241: Added
SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62241/#review188053
-----------------------------------------------------------
Looks good, just a few minor things to clean up.
src/tests/scheduler_http_api_tests.cpp
Lines 674 (patched)
<https://reviews.apache.org/r/62241/#comment265133>
s/an/a
src/tests/scheduler_http_api_tests.cpp
Lines 712-716 (patched)
<https://reviews.apache.org/r/62241/#comment265134>
Do you care about a `HEARTBEAT` even for this test?
src/tests/scheduler_http_api_tests.cpp
Lines 737-747 (patched)
<https://reviews.apache.org/r/62241/#comment265135>
Kill this? We don't need this to test if the downgrade worked.
- Anand Mazumdar
On Sept. 12, 2017, 12:59 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62241/
> -----------------------------------------------------------
>
> (Updated Sept. 12, 2017, 12:59 p.m.)
>
>
> Review request for mesos and Anand Mazumdar.
>
>
> Bugs: MESOS-7867
> https://issues.apache.org/jira/browse/MESOS-7867
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This test verifies that we are able to upgrade from a PID based
> framework to an HTTP framework and then downgrade back without
> restarting the master.
>
>
> Diffs
> -----
>
> src/tests/scheduler_http_api_tests.cpp cc03be0d2110f08023bf0ce181da37658737bd7c
>
>
> Diff: https://reviews.apache.org/r/62241/diff/1/
>
>
> Testing
> -------
>
> Ran `make check`. Verified that the test fails without the previous patch.
>
>
> Thanks,
>
> Ilya Pronin
>
>