mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 54081: Added `--pidfile` option to master and agent binaries.
Date Tue, 06 Dec 2016 22:42:09 GMT

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



Thanks Ilya!

Have you looked at other pidfile related libraries? Looks like BSD provides some functions
for this (they're also available on Linux):
https://www.freebsd.org/cgi/man.cgi?query=pidfile&sektion=3&manpath=FreeBSD+6.1-RELEASE
https://linux.die.net/man/3/pidfile

It would be great to provide an os-agnostic library within stout for this, e.g.:

```
pidfile::open()
pidfile::close()
pidfile::remove()
```

I haven't looked too deeply at what we should have these take and return, but I noticed the
use of RAII seemed unnecessary in your patch (only the .cpp implementation uses the exposed
RAII class? If so, perhaps we can just leave RAII out for now since the callers do not use
it). Also it would be great to which process is already running (which is provided by the
BSD/Linux functions). Also, another suggestion as a first step here is to avoid Windows support
entirely for now if we don't implement the locking aspect of this (as that seems necessary
for this to be safely usable?)

We could structure the patches like this:

(1) Addition of pidfile utilities to stout.
(2) Tests of the pidfile utilities in stout (should be possible?)
(3) Integration of the pidfile utilities into mesos (manually test since the logic will reside
in the mains?)

- Benjamin Mahler


On Nov. 28, 2016, 12:16 p.m., Ilya Pronin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54081/
> -----------------------------------------------------------
> 
> (Updated Nov. 28, 2016, 12:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-1648
>     https://issues.apache.org/jira/browse/MESOS-1648
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The PID file is created and kept locked while master / agent is running to prevent other
instances with the same PID file location from starting.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt d6e213686a44fbca0ed841594ce803151224a5a7 
>   src/Makefile.am dd1626d177b38a6613f18f32bb0668abbb5100e0 
>   src/common/pid_file.hpp PRE-CREATION 
>   src/common/pid_file.cpp PRE-CREATION 
>   src/logging/logging.cpp 70d66a5c396f709e8f27ad0d51315ed6d257f73b 
>   src/master/main.cpp fa7ba1310142a3bef71379ba37fded9b8390aae9 
>   src/slave/main.cpp 8010f8e229e2d820649750c9db0456ecd1b854d3 
>   src/tests/common/pid_file_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54081/diff/
> 
> 
> Testing
> -------
> 
> Added test to verify that PID file is created upon `PIDFile` object creation and deleted
upon its destruction. Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>


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