mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anand Mazumdar <an...@apache.org>
Subject Re: Review Request 62241: Added SchedulerHttpApiTest.UpdateHttpToPidSchedulerAndBack test.
Date Mon, 16 Oct 2017 18:42:28 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message