You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alexandra Sava <al...@gmail.com> on 2014/06/28 18:19:13 UTC
Review Request 23147: Rename slaves/frameworks activated/deactivated
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/
-----------------------------------------------------------
Review request for mesos, Adam B and Ben Mahler.
Bugs: MESOS-1188
https://issues.apache.org/jira/browse/MESOS-1188
Repository: mesos-git
Description
-------
The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
In order to make things look clear, rename the following:
* master.slaves.deactivated -> master.slaves.removed
* master.slaves.activated -> master.slaves.registered
* master.frameworks.activated -> master.frameworks.registered
* allocator.slaveDisconnect -> allocator.slaveDeactivate
* allocator.slaveReconnected -> allocator.slaveReactivated
Diffs
-----
src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
src/master/master.cpp 21b07c7f1f445beac29a7781cf441dd79b1b7fb5
src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
src/tests/master_authorization_tests.cpp 478041cdea533e548ca92c4b8e8c793554855969
src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
Diff: https://reviews.apache.org/r/23147/diff/
Testing
-------
Thanks,
Alexandra Sava
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review47751
-----------------------------------------------------------
Ship it!
Thanks Alexandra, this looks good to me. I am ok with this correcting the old metric values and leaving the names as they are.
I will let Adam get this committed for you!
Adam, feel free to pull down her patch and add the names in the TODOs to avoid another unnecessary review cycle.
- Ben Mahler
On July 11, 2014, 1:28 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 1:28 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
> src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
> src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review47691
-----------------------------------------------------------
Ship it!
Looks great! Please add your id to the TODOs and rebase against the latest master (if you haven't already), and I would be happy to commit this for you.
src/master/http.cpp
<https://reviews.apache.org/r/23147/#comment83838>
Please add your userid to the TODO: // TODO(alexandra.sava) Add... (Same for the other TODOs in here.)
src/master/http.cpp
<https://reviews.apache.org/r/23147/#comment83839>
While I agree that these are the correct new values for the new interpretations of the activated/deactivated terms, I hope nobody was depending on the old meanings of these stats.
I could imagine keeping activated|deactivated_slaves to the old meaning (registered/removed) and adding new active|inactive_slaves stats, but that would be too confusing to a first-time stats viewer. Better to change them and document.
That said, I don't know if it would be better to document it in docs/upgrades.md (mostly about the upgrade process) or just put a note in the CHANGELOG or release notes blog. I'll ask around, but that shouldn't stop us from committing this.
src/master/http.cpp
<https://reviews.apache.org/r/23147/#comment83840>
Ditto.
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment83841>
// TODO(alexandra.sava): Count...
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment83842>
// TODO(alexandra.sava): Add...
- Adam B
On July 11, 2014, 6:28 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 6:28 a.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
> src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
> src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review47649
-----------------------------------------------------------
Patch looks great!
Reviews applied: [23147]
All tests passed.
- Mesos ReviewBot
On July 11, 2014, 1:28 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 1:28 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
> src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
> src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Alexandra Sava <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/
-----------------------------------------------------------
(Updated July 11, 2014, 1:28 p.m.)
Review request for mesos, Adam B and Ben Mahler.
Bugs: MESOS-1188
https://issues.apache.org/jira/browse/MESOS-1188
Repository: mesos-git
Description
-------
The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
In order to make things look clear, rename the following:
* master.slaves.deactivated -> master.slaves.removed
* master.slaves.activated -> master.slaves.registered
* master.frameworks.activated -> master.frameworks.registered
* allocator.slaveDisconnect -> allocator.slaveDeactivate
* allocator.slaveReconnected -> allocator.slaveReactivated
Diffs (updated)
-----
src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
Diff: https://reviews.apache.org/r/23147/diff/
Testing
-------
Thanks,
Alexandra Sava
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review47421
-----------------------------------------------------------
Bad patch!
Reviews applied: [23147]
Failed command: git apply --index 23147.patch
Error:
error: patch failed: src/master/http.cpp:351
error: src/master/http.cpp: patch does not apply
- Mesos ReviewBot
On July 7, 2014, 7:05 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 7, 2014, 7:05 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 251b699ef971d69190bb2269ec8679067383e546
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Alexandra Sava <al...@gmail.com>.
> On July 8, 2014, 8:53 a.m., Adam B wrote:
> > src/master/http.cpp, lines 356-358
> > <https://reviews.apache.org/r/23147/diff/2/?file=624768#file624768line356>
> >
> > Add a comment here for when we can remove them (0.21 if this goes into 0.20?)
Ok so since I'm currently working on MESOS-1476 and I plan to name 'active slaves'/'deactive slaves' the slaves for which resource offers are / are not sent, we will need a statistic for that as well. Since we already have it but it's not counting the right thing, I will change it to count the right thing: the number of active slaves (slaves for which resource offers are sent). Currently, the only way slaves can get deactivated is when they are disconnected (please have a look @ Master::disconnect method). So, in order to count the deactivated slaves it's enough to count the disconnected slaves (there already is a method that counts this: Master::_slaves_inactive()).
Once MESOS-1476 gets commited, I will change _slaves_active method (which currently counts the disconnected slaves - which are also deactivated slaves) to also count the slaves that have been deactivated via HTTP POSTs. I will add a TODO for that.
Also, we need metrics and stats for "registered_slaves"/"removed_slaves". I will create a separate review for that. I will add just a TODO now.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review47434
-----------------------------------------------------------
On July 11, 2014, 1:28 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 1:28 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
> src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
> src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Adam B <ad...@mesosphere.io>.
> On July 8, 2014, 1:53 a.m., Adam B wrote:
> > src/master/http.cpp, lines 356-358
> > <https://reviews.apache.org/r/23147/diff/2/?file=624768#file624768line356>
> >
> > Add a comment here for when we can remove them (0.21 if this goes into 0.20?)
>
> Alexandra Sava wrote:
> Ok so since I'm currently working on MESOS-1476 and I plan to name 'active slaves'/'deactive slaves' the slaves for which resource offers are / are not sent, we will need a statistic for that as well. Since we already have it but it's not counting the right thing, I will change it to count the right thing: the number of active slaves (slaves for which resource offers are sent). Currently, the only way slaves can get deactivated is when they are disconnected (please have a look @ Master::disconnect method). So, in order to count the deactivated slaves it's enough to count the disconnected slaves (there already is a method that counts this: Master::_slaves_inactive()).
>
> Once MESOS-1476 gets commited, I will change _slaves_active method (which currently counts the disconnected slaves - which are also deactivated slaves) to also count the slaves that have been deactivated via HTTP POSTs. I will add a TODO for that.
>
> Also, we need metrics and stats for "registered_slaves"/"removed_slaves". I will create a separate review for that. I will add just a TODO now.
>
Sounds great!
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review47434
-----------------------------------------------------------
On July 11, 2014, 6:28 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 6:28 a.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
> src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
> src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review47434
-----------------------------------------------------------
src/master/allocator.hpp
<https://reviews.apache.org/r/23147/#comment83252>
Why not slaveActivated(), like frameworkActivated() above?
src/master/http.cpp
<https://reviews.apache.org/r/23147/#comment83253>
Add a comment here for when we can remove them (0.21 if this goes into 0.20?)
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment83254>
and deactivate it in the allocator
- Adam B
On July 7, 2014, 12:05 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 7, 2014, 12:05 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 251b699ef971d69190bb2269ec8679067383e546
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Alexandra Sava <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/
-----------------------------------------------------------
(Updated July 7, 2014, 7:05 p.m.)
Review request for mesos, Adam B and Ben Mahler.
Bugs: MESOS-1188
https://issues.apache.org/jira/browse/MESOS-1188
Repository: mesos-git
Description
-------
The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
In order to make things look clear, rename the following:
* master.slaves.deactivated -> master.slaves.removed
* master.slaves.activated -> master.slaves.registered
* master.frameworks.activated -> master.frameworks.registered
* allocator.slaveDisconnect -> allocator.slaveDeactivate
* allocator.slaveReconnected -> allocator.slaveReactivated
Diffs (updated)
-----
src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
src/master/master.cpp 251b699ef971d69190bb2269ec8679067383e546
src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
Diff: https://reviews.apache.org/r/23147/diff/
Testing
-------
Thanks,
Alexandra Sava
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Alexandra Sava <al...@gmail.com>.
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.hpp, line 316
> > <https://reviews.apache.org/r/23147/diff/1/?file=620067#file620067line316>
> >
> > deactivate
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.hpp, lines 852-854
> > <https://reviews.apache.org/r/23147/diff/1/?file=620067#file620067line852>
> >
> > // We mark a slave 'inactive' ...
> > bool active;
I have some concerns in changing this. As the comment says, a slave is marked as 'disconnected' when it is checkpointing. I would like to use the 'deactive' key for slaves for which no resource offers are being sent (this is for MESOS-1476 ticket).
In Master::statusUpdateAcknowledgement method, you can't send a status update ACK to a disconnected slave. Though, this should be possible for a 'deactivated' slave (for which no resource offers are being sent, but is currently running some tasks that had been lunched before the slave was marked as deactivated -> TODO for MESOS-1476 ticket ).
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46964
-----------------------------------------------------------
On June 28, 2014, 4:19 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated June 28, 2014, 4:19 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 21b07c7f1f445beac29a7781cf441dd79b1b7fb5
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 478041cdea533e548ca92c4b8e8c793554855969
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Adam B <ad...@mesosphere.io>.
> On June 29, 2014, 11:44 p.m., Adam B wrote:
> > src/master/master.hpp, lines 852-854
> > <https://reviews.apache.org/r/23147/diff/1/?file=620067#file620067line852>
> >
> > // We mark a slave 'inactive' ...
> > bool active;
>
> Alexandra Sava wrote:
> I have some concerns in changing this. As the comment says, a slave is marked as 'disconnected' when it is checkpointing. I would like to use the 'deactive' key for slaves for which no resource offers are being sent (this is for MESOS-1476 ticket).
>
> In Master::statusUpdateAcknowledgement method, you can't send a status update ACK to a disconnected slave. Though, this should be possible for a 'deactivated' slave (for which no resource offers are being sent, but is currently running some tasks that had been lunched before the slave was marked as deactivated -> TODO for MESOS-1476 ticket ).
>
Hmm. I opine that 'disconnected' is the proper term for a slave that has lost its connection to the master, and which will have to reregister upon reconnecting. Also, 'deactivated' is the proper term for a slave for which the allocator will no longer send any resource offers, and any outstanding offers will no longer be considered valid. I could even believe that disconnecting a slave with Master::disconnect(Slave) would involve deactivating it in the allocator with allocator->slaveDeactivated(). So far, the two terms/states have been tightly coupled. Now that you propose a separation of states (for MESOS-1476), you must be careful when considering one of the states to also consider the other. I think it's safe to say that a 'disconnected' slave should always also be 'deactivated', but that a slave can also be 'deactivated' manually, without getting disconnected first. A connected-but-deactivated slave may even be able to reactivate without needing to reauthenticate/reregister. But w
hat if a deactivated slave does try to reregister? Does it restart into deactivated state? Can you call KillTask on a connected-but-deactivated slave?
TLDR: You can keep 'bool disconnected' and even 'disconnect(Slave)' as is for now, but be very careful when introducing a separate 'deactivated' state for MESOS-1476.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46964
-----------------------------------------------------------
On July 7, 2014, 12:05 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 7, 2014, 12:05 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 251b699ef971d69190bb2269ec8679067383e546
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Adam B <ad...@mesosphere.io>.
> On June 29, 2014, 11:44 p.m., Adam B wrote:
> > src/master/master.hpp, lines 852-854
> > <https://reviews.apache.org/r/23147/diff/1/?file=620067#file620067line852>
> >
> > // We mark a slave 'inactive' ...
> > bool active;
>
> Alexandra Sava wrote:
> I have some concerns in changing this. As the comment says, a slave is marked as 'disconnected' when it is checkpointing. I would like to use the 'deactive' key for slaves for which no resource offers are being sent (this is for MESOS-1476 ticket).
>
> In Master::statusUpdateAcknowledgement method, you can't send a status update ACK to a disconnected slave. Though, this should be possible for a 'deactivated' slave (for which no resource offers are being sent, but is currently running some tasks that had been lunched before the slave was marked as deactivated -> TODO for MESOS-1476 ticket ).
>
>
> Adam B wrote:
> Hmm. I opine that 'disconnected' is the proper term for a slave that has lost its connection to the master, and which will have to reregister upon reconnecting. Also, 'deactivated' is the proper term for a slave for which the allocator will no longer send any resource offers, and any outstanding offers will no longer be considered valid. I could even believe that disconnecting a slave with Master::disconnect(Slave) would involve deactivating it in the allocator with allocator->slaveDeactivated(). So far, the two terms/states have been tightly coupled. Now that you propose a separation of states (for MESOS-1476), you must be careful when considering one of the states to also consider the other. I think it's safe to say that a 'disconnected' slave should always also be 'deactivated', but that a slave can also be 'deactivated' manually, without getting disconnected first. A connected-but-deactivated slave may even be able to reactivate without needing to reauthenticate/reregister.
But what if a deactivated slave does try to reregister? Does it restart into deactivated state? Can you call KillTask on a connected-but-deactivated slave?
>
> TLDR: You can keep 'bool disconnected' and even 'disconnect(Slave)' as is for now, but be very careful when introducing a separate 'deactivated' state for MESOS-1476.
>
> Alexandra Sava wrote:
> If a connected-deactivated slave tries to re-register, it will still be deactivated. So the state of the slave (activated/deactivated) is kept in the registry. The master will consider a slave as deactivated as long as that slave will be in the registry. If for some reason, the slave is removed from the registry, and after that, at some point it will re-register, the master will consider it activated.
>
> A connected-but-deactivated slave is just like a normal slave but the master will no longer send resource offers that belongs to it. If an operator deactivates a slave, that slave will continue running the tasks that had been lunched on it before deactivation. So yes.. you can KillTask on a connected-but-deactivated slave.
SGTM. Just want to make sure we're considering all these cases now that we're splitting the 'deactivated' state into two states.
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46964
-----------------------------------------------------------
On July 11, 2014, 6:28 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 6:28 a.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
> src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
> src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Alexandra Sava <al...@gmail.com>.
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/http.cpp, line 351
> > <https://reviews.apache.org/r/23147/diff/1/?file=620066#file620066line351>
> >
> > There could be upgrade issues for anyone using hardcoded keys to read from the stats endpoint. I'm not sure how important that is, but I can think of two approaches:
> > 1) Just document the change.
> > 2) Document the change, and store these stats at the old keys as well as the new ones. The old keys can be deprecated in a future release.
The second approach seems ok to me. The first one does not fix the upgrade issues you mentioned.
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/http.cpp, line 354
> > <https://reviews.apache.org/r/23147/diff/1/?file=620066#file620066line354>
> >
> > I wonder if "total_schedulers" is the correct term for this. Perhaps "registered_schedulers" would be more explicit?
I'm not sure about that. Other opinions are welcomed.
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/http.cpp, lines 354-355
> > <https://reviews.apache.org/r/23147/diff/1/?file=620066#file620066line354>
> >
> > BTW, why are these "schedulers" and not "frameworks"? Should we rename them while we're at it?
Well, maybe because each framework has it's own scheduler ?! Other opinions are welcomed.
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.hpp, line 746
> > <https://reviews.apache.org/r/23147/diff/1/?file=620067#file620067line746>
> >
> > active(true),
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, lines 768-770
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line768>
> >
> > } else if (slave->active) {
> > // Checkpointing slaves can just be deactivated.
> > deactivate(slave);
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, line 1598
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line1598>
> >
> > deactivate(Slave*)
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, line 1602
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line1602>
> >
> > Deactivating
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, line 1604
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line1604>
> >
> > deactivated
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, line 1605
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line1605>
> >
> > slave->active = false;
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, line 2746
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line2746>
> >
> > if (!slave->active) {
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, line 2747
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line2747>
> >
> > deactivated
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, line 2752
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line2752>
> >
> > deactivated
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, lines 2927-2932
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line2927>
> >
> > // If this is a deactivated slave...
> > if (!slave->active) {
> > slave->active = true;
Please see the comment for 'bool disconnected' @ master.hpp:854
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.cpp, lines 3390-3391
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line3390>
> >
> > I wonder if we should also be checking if (!slaves.registered[slaveId]->active)
This is already done at line 3410 with a different log message.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46964
-----------------------------------------------------------
On July 7, 2014, 7:05 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 7, 2014, 7:05 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 251b699ef971d69190bb2269ec8679067383e546
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Alexandra Sava <al...@gmail.com>.
> On June 30, 2014, 6:44 a.m., Adam B wrote:
> > src/master/master.hpp, lines 852-854
> > <https://reviews.apache.org/r/23147/diff/1/?file=620067#file620067line852>
> >
> > // We mark a slave 'inactive' ...
> > bool active;
>
> Alexandra Sava wrote:
> I have some concerns in changing this. As the comment says, a slave is marked as 'disconnected' when it is checkpointing. I would like to use the 'deactive' key for slaves for which no resource offers are being sent (this is for MESOS-1476 ticket).
>
> In Master::statusUpdateAcknowledgement method, you can't send a status update ACK to a disconnected slave. Though, this should be possible for a 'deactivated' slave (for which no resource offers are being sent, but is currently running some tasks that had been lunched before the slave was marked as deactivated -> TODO for MESOS-1476 ticket ).
>
>
> Adam B wrote:
> Hmm. I opine that 'disconnected' is the proper term for a slave that has lost its connection to the master, and which will have to reregister upon reconnecting. Also, 'deactivated' is the proper term for a slave for which the allocator will no longer send any resource offers, and any outstanding offers will no longer be considered valid. I could even believe that disconnecting a slave with Master::disconnect(Slave) would involve deactivating it in the allocator with allocator->slaveDeactivated(). So far, the two terms/states have been tightly coupled. Now that you propose a separation of states (for MESOS-1476), you must be careful when considering one of the states to also consider the other. I think it's safe to say that a 'disconnected' slave should always also be 'deactivated', but that a slave can also be 'deactivated' manually, without getting disconnected first. A connected-but-deactivated slave may even be able to reactivate without needing to reauthenticate/reregister.
But what if a deactivated slave does try to reregister? Does it restart into deactivated state? Can you call KillTask on a connected-but-deactivated slave?
>
> TLDR: You can keep 'bool disconnected' and even 'disconnect(Slave)' as is for now, but be very careful when introducing a separate 'deactivated' state for MESOS-1476.
If a connected-deactivated slave tries to re-register, it will still be deactivated. So the state of the slave (activated/deactivated) is kept in the registry. The master will consider a slave as deactivated as long as that slave will be in the registry. If for some reason, the slave is removed from the registry, and after that, at some point it will re-register, the master will consider it activated.
A connected-but-deactivated slave is just like a normal slave but the master will no longer send resource offers that belongs to it. If an operator deactivates a slave, that slave will continue running the tasks that had been lunched on it before deactivation. So yes.. you can KillTask on a connected-but-deactivated slave.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46964
-----------------------------------------------------------
On July 11, 2014, 1:28 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 1:28 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
> src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
> src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46964
-----------------------------------------------------------
Looks great, Alexandra! Much more readable now. I know it's a lot of code to go through, so I appreciate your effort.
There are still a couple more related variables/methods left to rename (Master::Slave::disconnected, Master::disconnect(Slave*), InvokeSlaveReconnected()), and some documentation updates. Once that is complete, this will be ready to commit. Thanks!
src/master/allocator.hpp
<https://reviews.apache.org/r/23147/#comment82538>
s/reconnected/reactivated/
src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/23147/#comment82539>
s/until they reactivated/until they are reactivated/
src/master/http.cpp
<https://reviews.apache.org/r/23147/#comment82541>
There could be upgrade issues for anyone using hardcoded keys to read from the stats endpoint. I'm not sure how important that is, but I can think of two approaches:
1) Just document the change.
2) Document the change, and store these stats at the old keys as well as the new ones. The old keys can be deprecated in a future release.
src/master/http.cpp
<https://reviews.apache.org/r/23147/#comment82540>
I wonder if "total_schedulers" is the correct term for this. Perhaps "registered_schedulers" would be more explicit?
src/master/http.cpp
<https://reviews.apache.org/r/23147/#comment82542>
BTW, why are these "schedulers" and not "frameworks"? Should we rename them while we're at it?
src/master/http.cpp
<https://reviews.apache.org/r/23147/#comment82543>
Again, we'll need to document the change and possibly deprecate the old keys.
src/master/master.hpp
<https://reviews.apache.org/r/23147/#comment82558>
Remove this TODO
src/master/master.hpp
<https://reviews.apache.org/r/23147/#comment82559>
deactivate
src/master/master.hpp
<https://reviews.apache.org/r/23147/#comment82560>
// A registered slave.
Or just remove the comment. It doesn't really add much.
src/master/master.hpp
<https://reviews.apache.org/r/23147/#comment82561>
active(true),
src/master/master.hpp
<https://reviews.apache.org/r/23147/#comment82562>
// We mark a slave 'inactive' ...
bool active;
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82545>
} else if (slave->active) {
// Checkpointing slaves can just be deactivated.
deactivate(slave);
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82546>
deactivate(Slave*)
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82547>
Deactivating
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82548>
deactivated
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82549>
slave->active = false;
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82550>
if (!slave->active) {
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82551>
deactivated
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82552>
deactivated
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82553>
// If this is a deactivated slave...
if (!slave->active) {
slave->active = true;
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82554>
I wonder if we should also be checking if (!slaves.registered[slaveId]->active)
src/tests/mesos.hpp
<https://reviews.apache.org/r/23147/#comment82556>
InvokeSlaveReactivated
src/tests/mesos.hpp
<https://reviews.apache.org/r/23147/#comment82557>
InvokeSlaveReactivated
- Adam B
On June 28, 2014, 9:19 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated June 28, 2014, 9:19 a.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 21b07c7f1f445beac29a7781cf441dd79b1b7fb5
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 478041cdea533e548ca92c4b8e8c793554855969
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46950
-----------------------------------------------------------
Patch looks great!
Reviews applied: [23147]
All tests passed.
- Mesos ReviewBot
On June 28, 2014, 4:19 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated June 28, 2014, 4:19 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 21b07c7f1f445beac29a7781cf441dd79b1b7fb5
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 478041cdea533e548ca92c4b8e8c793554855969
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Alexandra Sava <al...@gmail.com>.
> On June 30, 2014, 4:13 p.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 4354
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line4354>
> >
> > have you considered renaming the metrics?
>
> Alexandra Sava wrote:
> How do you suggest to rename them? slaves_registered is not ok in my opinion because the opposite is slaves_unregistered. I can't rename slaves_inactive into slaves_unregistered because this method does not return the number of unregistered slaves.
> I think slaves_connected/slaves_disconnected is more suitable for this context, but there currently is a debate whether to rename 'slave->disconnected' into 'slave->active' or not.
>
> Dominic Hamon wrote:
> my preference is that the metrics reflect the underlying variable, so when the debate is settled it should be clear.
So this methods should count the number of activated slaves (which are slaves for which no resource offers are sent, according to MESOS-1476). Currently slaves are marked as deactivated when they are disconnected (please have a look @ Master::disconnect method). Since slaves can not be deactivated in any other way (just for now), it is correct what this method does (counts the number of disconnected slaves).
I'm currently working on MESOS-1476 and when it will be committed, I will change this method to also consider the slaves deactivated by HTTP POSTS. I will add a TODO there.
I will create metrics for registered and removed slaves in another review.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46985
-----------------------------------------------------------
On July 11, 2014, 1:28 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 11, 2014, 1:28 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 4fba007bfb9909056dc85f9dc04483994d662740
> src/master/master.hpp 8641f2dfe711481133869f876715b56728dc1bc0
> src/master/master.cpp 86b147fce153fe3a241dbd841e033f2b7ca07b01
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Dominic Hamon <dh...@twopensource.com>.
> On June 30, 2014, 9:13 a.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 4354
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line4354>
> >
> > have you considered renaming the metrics?
>
> Alexandra Sava wrote:
> How do you suggest to rename them? slaves_registered is not ok in my opinion because the opposite is slaves_unregistered. I can't rename slaves_inactive into slaves_unregistered because this method does not return the number of unregistered slaves.
> I think slaves_connected/slaves_disconnected is more suitable for this context, but there currently is a debate whether to rename 'slave->disconnected' into 'slave->active' or not.
my preference is that the metrics reflect the underlying variable, so when the debate is settled it should be clear.
- Dominic
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46985
-----------------------------------------------------------
On July 7, 2014, 12:05 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 7, 2014, 12:05 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 251b699ef971d69190bb2269ec8679067383e546
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Alexandra Sava <al...@gmail.com>.
> On June 30, 2014, 4:13 p.m., Dominic Hamon wrote:
> > src/master/master.cpp, line 4354
> > <https://reviews.apache.org/r/23147/diff/1/?file=620068#file620068line4354>
> >
> > have you considered renaming the metrics?
How do you suggest to rename them? slaves_registered is not ok in my opinion because the opposite is slaves_unregistered. I can't rename slaves_inactive into slaves_unregistered because this method does not return the number of unregistered slaves.
I think slaves_connected/slaves_disconnected is more suitable for this context, but there currently is a debate whether to rename 'slave->disconnected' into 'slave->active' or not.
- Alexandra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46985
-----------------------------------------------------------
On July 7, 2014, 7:05 p.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated July 7, 2014, 7:05 p.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 251b699ef971d69190bb2269ec8679067383e546
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 0fdf464cc4a562afec276ec604205af3b56636de
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>
Re: Review Request 23147: Rename slaves/frameworks activated/deactivated
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23147/#review46985
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/23147/#comment82581>
have you considered renaming the metrics?
- Dominic Hamon
On June 28, 2014, 9:19 a.m., Alexandra Sava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23147/
> -----------------------------------------------------------
>
> (Updated June 28, 2014, 9:19 a.m.)
>
>
> Review request for mesos, Adam B and Ben Mahler.
>
>
> Bugs: MESOS-1188
> https://issues.apache.org/jira/browse/MESOS-1188
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> The existing terminology is confusing both for "slaves.deactivated" and "frameworks.activated". Currently a deactivated slave actually represents a removed/shutdown slave and "frameworks.activated" map holds both activated and deactivated frameworks.
> In order to make things look clear, rename the following:
> * master.slaves.deactivated -> master.slaves.removed
> * master.slaves.activated -> master.slaves.registered
> * master.frameworks.activated -> master.frameworks.registered
> * allocator.slaveDisconnect -> allocator.slaveDeactivate
> * allocator.slaveReconnected -> allocator.slaveReactivated
>
>
> Diffs
> -----
>
> src/master/allocator.hpp 1cd573477b609bb69264f16156a4004ecac672a7
> src/master/constants.hpp 2daa9b004ab0cc79773730350369f66315356cad
> src/master/constants.cpp e9e5e67f890f3399c24637c0f021d656dfe51118
> src/master/hierarchical_allocator_process.hpp 1765e7035bdda4c28e79d74c92e77dcc99759001
> src/master/http.cpp 5d869767cd3ed48aae1e702e8d014a37ef371123
> src/master/master.hpp 5fef35406c2ce2ad11e030aa7752eb691aab5857
> src/master/master.cpp 21b07c7f1f445beac29a7781cf441dd79b1b7fb5
> src/tests/fault_tolerance_tests.cpp ac65050bec5720b982f53d4dd6797cc3dee285dc
> src/tests/master_authorization_tests.cpp 478041cdea533e548ca92c4b8e8c793554855969
> src/tests/mesos.hpp ae38a13d8b329f6e27813776e0d2f2b56605d0eb
> src/tests/slave_recovery_tests.cpp 582f52d73eba0e3ab089ec573d9a6c43bff0339e
> src/webui/master/static/home.html ce8ca192235c224715c01fef0b8ddb187dc0a827
> src/webui/master/static/js/controllers.js 41a70a80442501a2bf7b217939dbe504662941d2
>
> Diff: https://reviews.apache.org/r/23147/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alexandra Sava
>
>