You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ben Mahler <be...@gmail.com> on 2012/10/19 02:44:00 UTC

Review Request: Replacing executor run directory numbers with uuids.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/
-----------------------------------------------------------

Review request for mesos, Benjamin Hindman and Vinod Kone.


Description
-------

This replaces our run directory numbers with the executor uuids.

Ex:
.../executorFoo/runs/0
.../executorFoo/runs/1111-1111-1111-1111

This ensures we don't ever write to an old directory that was gc'ed.


Diffs
-----

  src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
  src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
  src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
  src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
  src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
  src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
  third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 

Diff: https://reviews.apache.org/r/7658/diff/


Testing
-------

Updated tests + make check.


Thanks,

Ben Mahler


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 25, 2012, 5:58 p.m., Vinod Kone wrote:
> >

Using the absolute path actually makes my life much easier, I just assumed we may want to hide it.
The webui could just use the executor.directory directly, instead of hardcoding the path format.
Users of the webui would also be able to know the full path to their files, which should help them debug.

Does this sound good? If so, I'd update the subsequent review in this chain: https://reviews.apache.org/r/7642/


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/#review12779
-----------------------------------------------------------


On Oct. 25, 2012, 1:08 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7658/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2012, 1:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This replaces our run directory numbers with the executor uuids.
> 
> Ex:
> .../executorFoo/runs/0
> .../executorFoo/runs/1111-1111-1111-1111
> 
> This ensures we don't ever write to an old directory that was gc'ed.
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
>   src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
>   src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
>   src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
>   src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
>   src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
>   third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 
> 
> Diff: https://reviews.apache.org/r/7658/diff/
> 
> 
> Testing
> -------
> 
> Updated tests + make check.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Vinod Kone <vi...@gmail.com>.

> On Oct. 25, 2012, 5:58 p.m., Vinod Kone wrote:
> >
> 
> Ben Mahler wrote:
>     Using the absolute path actually makes my life much easier, I just assumed we may want to hide it.
>     The webui could just use the executor.directory directly, instead of hardcoding the path format.
>     Users of the webui would also be able to know the full path to their files, which should help them debug.
>     
>     Does this sound good? If so, I'd update the subsequent review in this chain: https://reviews.apache.org/r/7642/

sgtm


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/#review12779
-----------------------------------------------------------


On Oct. 25, 2012, 1:08 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7658/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2012, 1:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This replaces our run directory numbers with the executor uuids.
> 
> Ex:
> .../executorFoo/runs/0
> .../executorFoo/runs/1111-1111-1111-1111
> 
> This ensures we don't ever write to an old directory that was gc'ed.
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
>   src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
>   src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
>   src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
>   src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
>   src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
>   third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 
> 
> Diff: https://reviews.apache.org/r/7658/diff/
> 
> 
> Testing
> -------
> 
> Updated tests + make check.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/#review12779
-----------------------------------------------------------

Ship it!



src/slave/slave.cpp
<https://reviews.apache.org/r/7658/#comment27335>

    aah, i wish we were able to leverage the path templates here. but, i guess, since those templates give out absolute paths, its hard for you to utilize. thoughts?


- Vinod Kone


On Oct. 25, 2012, 1:08 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7658/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2012, 1:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This replaces our run directory numbers with the executor uuids.
> 
> Ex:
> .../executorFoo/runs/0
> .../executorFoo/runs/1111-1111-1111-1111
> 
> This ensures we don't ever write to an old directory that was gc'ed.
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
>   src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
>   src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
>   src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
>   src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
>   src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
>   third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 
> 
> Diff: https://reviews.apache.org/r/7658/diff/
> 
> 
> Testing
> -------
> 
> Updated tests + make check.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/
-----------------------------------------------------------

(Updated Oct. 25, 2012, 1:08 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Updated line wrapping of path constants.


Description
-------

This replaces our run directory numbers with the executor uuids.

Ex:
.../executorFoo/runs/0
.../executorFoo/runs/1111-1111-1111-1111

This ensures we don't ever write to an old directory that was gc'ed.


Diffs (updated)
-----

  src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
  src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
  src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
  src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
  src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
  src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
  third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 

Diff: https://reviews.apache.org/r/7658/diff/


Testing
-------

Updated tests + make check.


Thanks,

Ben Mahler


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/
-----------------------------------------------------------

(Updated Oct. 25, 2012, 12:24 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Also tried to make sure we don't refer to 'uuid's as 'run's.


Description
-------

This replaces our run directory numbers with the executor uuids.

Ex:
.../executorFoo/runs/0
.../executorFoo/runs/1111-1111-1111-1111

This ensures we don't ever write to an old directory that was gc'ed.


Diffs (updated)
-----

  src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
  src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
  src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
  src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
  src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
  src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
  third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 

Diff: https://reviews.apache.org/r/7658/diff/


Testing
-------

Updated tests + make check.


Thanks,

Ben Mahler


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 24, 2012, 10:06 p.m., Benjamin Hindman wrote:
> > src/slave/paths.hpp, line 55
> > <https://reviews.apache.org/r/7658/diff/3/?file=178737#file178737line55>
> >
> >     I think wrapping stuff like this to all fit on the newline is easier to read.

Agreed, I just didn't like the inconsistency with the other constants, should I wrap them all too?


> On Oct. 24, 2012, 10:06 p.m., Benjamin Hindman wrote:
> > src/slave/paths.hpp, line 210
> > <https://reviews.apache.org/r/7658/diff/3/?file=178737#file178737line210>
> >
> >     Hmm, maybe we shouldn't jump the gun with conclusions here and just print the error. Why were you thinking the possible hypothesis would be useful?

Dropped.


> On Oct. 24, 2012, 10:06 p.m., Benjamin Hindman wrote:
> > third_party/libprocess/include/stout/fs.hpp, lines 31-32
> > <https://reviews.apache.org/r/7658/diff/3/?file=178743#file178743line31>
> >
> >     I like 'original' and 'link' better than 'path1' and 'path2'. ;)

I see you read up on your syscalls during reviews ;)


> On Oct. 24, 2012, 10:06 p.m., Benjamin Hindman wrote:
> > src/slave/paths.hpp, line 24
> > <https://reviews.apache.org/r/7658/diff/3/?file=178737#file178737line24>
> >
> >     I've tried to just include "logging/logging.hpp" instead of glog explicitly. I do this in the event we add other logging abstractions I'd rather just include a single thing. Just FYI.

Flipped to logging, for consistency.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/#review12724
-----------------------------------------------------------


On Oct. 22, 2012, 7:16 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7658/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2012, 7:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This replaces our run directory numbers with the executor uuids.
> 
> Ex:
> .../executorFoo/runs/0
> .../executorFoo/runs/1111-1111-1111-1111
> 
> This ensures we don't ever write to an old directory that was gc'ed.
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
>   src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
>   src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
>   src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
>   src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
>   src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
>   third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 
> 
> Diff: https://reviews.apache.org/r/7658/diff/
> 
> 
> Testing
> -------
> 
> Updated tests + make check.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Oct. 24, 2012, 10:06 p.m., Benjamin Hindman wrote:
> > src/slave/paths.hpp, line 55
> > <https://reviews.apache.org/r/7658/diff/3/?file=178737#file178737line55>
> >
> >     I think wrapping stuff like this to all fit on the newline is easier to read.
> 
> Ben Mahler wrote:
>     Agreed, I just didn't like the inconsistency with the other constants, should I wrap them all too?

Please!


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/#review12724
-----------------------------------------------------------


On Oct. 25, 2012, 12:24 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7658/
> -----------------------------------------------------------
> 
> (Updated Oct. 25, 2012, 12:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This replaces our run directory numbers with the executor uuids.
> 
> Ex:
> .../executorFoo/runs/0
> .../executorFoo/runs/1111-1111-1111-1111
> 
> This ensures we don't ever write to an old directory that was gc'ed.
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
>   src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
>   src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
>   src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
>   src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
>   src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
>   third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 
> 
> Diff: https://reviews.apache.org/r/7658/diff/
> 
> 
> Testing
> -------
> 
> Updated tests + make check.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/#review12724
-----------------------------------------------------------

Ship it!



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment27119>

    I've tried to just include "logging/logging.hpp" instead of glog explicitly. I do this in the event we add other logging abstractions I'd rather just include a single thing. Just FYI.



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment27118>

    We include stout as though it's an external library (because it really is), so using <> instead of "".



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment27120>

    I think wrapping stuff like this to all fit on the newline is easier to read.



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment27124>

    Hmm, maybe we shouldn't jump the gun with conclusions here and just print the error. Why were you thinking the possible hypothesis would be useful? 



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment27123>

    -2



src/slave/slave.hpp
<https://reviews.apache.org/r/7658/#comment27135>

    -2



src/slave/slave.cpp
<https://reviews.apache.org/r/7658/#comment27137>

    Please include a brief comment about why you are creating a UUID.



src/tests/slave_state_tests.cpp
<https://reviews.apache.org/r/7658/#comment27136>

    Same thing for including stout here, not sure why I missed this in a previous review.



third_party/libprocess/include/stout/fs.hpp
<https://reviews.apache.org/r/7658/#comment27138>

    I like 'original' and 'link' better than 'path1' and 'path2'. ;)


- Benjamin Hindman


On Oct. 22, 2012, 7:16 p.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7658/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2012, 7:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This replaces our run directory numbers with the executor uuids.
> 
> Ex:
> .../executorFoo/runs/0
> .../executorFoo/runs/1111-1111-1111-1111
> 
> This ensures we don't ever write to an old directory that was gc'ed.
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
>   src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
>   src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
>   src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
>   src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
>   src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
>   third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 
> 
> Diff: https://reviews.apache.org/r/7658/diff/
> 
> 
> Testing
> -------
> 
> Updated tests + make check.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/
-----------------------------------------------------------

(Updated Oct. 22, 2012, 7:16 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Vinod's review.


Description
-------

This replaces our run directory numbers with the executor uuids.

Ex:
.../executorFoo/runs/0
.../executorFoo/runs/1111-1111-1111-1111

This ensures we don't ever write to an old directory that was gc'ed.


Diffs (updated)
-----

  src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
  src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
  src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
  src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
  src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
  src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
  third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 

Diff: https://reviews.apache.org/r/7658/diff/


Testing
-------

Updated tests + make check.


Thanks,

Ben Mahler


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 19, 2012, 6:46 p.m., Vinod Kone wrote:
> > src/slave/paths.hpp, line 211
> > <https://reviews.apache.org/r/7658/diff/2/?file=177955#file177955line211>
> >
> >     how do you like 'current' instead of 'latest'? i think that's more idiomatic?

Discussed with benh and we decided on latest since "latest" makes sense regardless of whether the "latest" executor is running. Whereas "current" is a bit confusing when there's no executor running.


> On Oct. 19, 2012, 6:46 p.m., Vinod Kone wrote:
> > src/slave/paths.hpp, line 217
> > <https://reviews.apache.org/r/7658/diff/2/?file=177955#file177955line217>
> >
> >     s/mkdir/symlink/

nice catch!


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/#review12616
-----------------------------------------------------------


On Oct. 19, 2012, 12:46 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7658/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2012, 12:46 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This replaces our run directory numbers with the executor uuids.
> 
> Ex:
> .../executorFoo/runs/0
> .../executorFoo/runs/1111-1111-1111-1111
> 
> This ensures we don't ever write to an old directory that was gc'ed.
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
>   src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
>   src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
>   src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
>   src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
>   src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
>   third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 
> 
> Diff: https://reviews.apache.org/r/7658/diff/
> 
> 
> Testing
> -------
> 
> Updated tests + make check.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/#review12616
-----------------------------------------------------------



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment26927>

    s/CHECK/CHECK./



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment26929>

    kill line



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment26932>

    how do you like 'current' instead of 'latest'? i think that's more idiomatic?



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment26930>

    s/mkdir/symlink/



src/slave/paths.hpp
<https://reviews.apache.org/r/7658/#comment26931>

    s/mkdir/symlink/



src/slave/state.hpp
<https://reviews.apache.org/r/7658/#comment26933>

    Can you please add a TODO(vinod) here for keeping track of latest.



src/slave/state.cpp
<https://reviews.apache.org/r/7658/#comment26934>

    you need to ignore 'latest' when you loop through all runs here.
    
    foreach (const string& path, runs.get()) {
    
      //TODO(vinod): Store latest UUID in the state.
       if (path == "latest") {
        continue;
      }
       
    }



src/tests/slave_state_tests.cpp
<https://reviews.apache.org/r/7658/#comment26936>

    Why not use 'createExecutorWorkDirectory'?



third_party/libprocess/include/stout/fs.hpp
<https://reviews.apache.org/r/7658/#comment26937>

    thank you :)



third_party/libprocess/include/stout/fs.hpp
<https://reviews.apache.org/r/7658/#comment26938>

    kill line


- Vinod Kone


On Oct. 19, 2012, 12:46 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7658/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2012, 12:46 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This replaces our run directory numbers with the executor uuids.
> 
> Ex:
> .../executorFoo/runs/0
> .../executorFoo/runs/1111-1111-1111-1111
> 
> This ensures we don't ever write to an old directory that was gc'ed.
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
>   src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
>   src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
>   src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
>   src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
>   src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
>   third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 
> 
> Diff: https://reviews.apache.org/r/7658/diff/
> 
> 
> Testing
> -------
> 
> Updated tests + make check.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Replacing executor run directory numbers with uuids.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7658/
-----------------------------------------------------------

(Updated Oct. 19, 2012, 12:46 a.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Changes
-------

Updated the attached path, we don't want the run in there just yet. We'll want that when I send out another change for the webui.


Description
-------

This replaces our run directory numbers with the executor uuids.

Ex:
.../executorFoo/runs/0
.../executorFoo/runs/1111-1111-1111-1111

This ensures we don't ever write to an old directory that was gc'ed.


Diffs (updated)
-----

  src/slave/paths.hpp 48a7be0ae33785206ebb1985647178224d6bcfb8 
  src/slave/slave.hpp 343c353027dbd2a7c1be8cee99a1d59367169177 
  src/slave/slave.cpp 5af7464aae17c00a0e707421982d7cb055aabc6c 
  src/slave/state.hpp dcadd511063584cde51f533e2120f4eab5145fd2 
  src/slave/state.cpp c8406fbb5586bca5b7f169a2ef937c958626dee1 
  src/tests/slave_state_tests.cpp 0e232ff3ed773f3bdccc05e716b66d106d80fb2f 
  third_party/libprocess/include/stout/fs.hpp 1516d0b6fa6e0ced26bb08ab6a4fadf28232124f 

Diff: https://reviews.apache.org/r/7658/diff/


Testing
-------

Updated tests + make check.


Thanks,

Ben Mahler