mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 60821: Introduced a "no sender" UPID.
Date Fri, 14 Jul 2017 22:02:19 GMT


> On July 12, 2017, 5:27 p.m., James Peach wrote:
> > I think that a better approach is to define a static `UPID UPID::anonymous()` function
that returns a well-defined anonymous UPID for the system. Remove all the sending functions
that don't specify a `from` UPID and force them to explicitly pass the anonymous UPID. This
makes the used of anonymous message sends explicit which I think is preferable to this implicit
approach.
> 
> Jiang Yan Xu wrote:
>     Here's my thinking:
>     
>     - In libprocess the PID is usually not constructed directly, aside from being parsed
but that's also hidden from the users of the library. Users of libprocess tend to get the
PID from the process that owns that PID or from `spawn`. Afterwards it is used as an opaque
ID. Therefore I feel like more consistent with the current architecture to not expose it at
all. Having to know to use a speical ID is less straighfoward than calling a method that doesn't
take a `from` IMO.
>     
>     - Right now the PID is just a simple data struct and not guarded by a `process::initialize()`
call. This special UPID and PID uses the value of `__address__`, which initialized in `process::initialize()`.
Therefore even if we expose this anonymous / no sender PID directly, I feel it's probably
going to be constructed by something other than a static UPID method. This is a minor point
though.
> 
> James Peach wrote:
>     * Anything that calls `post` is going to have to initialize `libprocess`, so initializing
`libprocess` when we generate the anonymous UPID seems OK too. `UPID process::anonymous()`.
>     * Sending anonymous messages is a special case, so it should be explicit. `post`
is also a special case since you already have to specify a target UPID. I argue that explicit
is better than implicit and 1 way to `post` is better than 2.

Yeah I think the main thing I am arguing for here is that as you said, the purpose of this
is to support "Sending anonymous messages / message with no sender address". IMO the sentence
from the user's POV translates to a method (and we already have this method), not an UPID.

In othe words, this special [process::post](https://github.com/apache/mesos/blob/5645fddf07eab0fcd195a3514e9e5ad3ef25628d/3rdparty/libprocess/include/process/process.hpp#L626-L637)
overload serves a special purpose that
- We currently have a use case directly
- I don't see how it could be incorrectly used for other unintended ways.


In contrast, exposing a special UPID to user:
- I don't see how the UPID can be used for another legit purpose.
- I can see it could used incorrectly by the user.


I do support explictness, but I think saying a method "sends message with no sender address"
is pretty explicit too and without exposing too much flexibility and a concept that could
be hard(er) to undertand (than this).

Having said that, I am not adamant on the current design. Given another vote/idea I can switch.


- Jiang Yan


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


On July 12, 2017, 5:04 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60821/
> -----------------------------------------------------------
> 
> (Updated July 12, 2017, 5:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-7786
>     https://issues.apache.org/jira/browse/MESOS-7786
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The immediate use of this is to fix MESOS-7753 but I feel this is a general concept that's
defined in similar systems (e.g., akka).
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp b4d7791782a5d9e10fa44489057c8504d594bab2 
>   3rdparty/libprocess/src/tests/process_tests.cpp c6109547d04bd06e9395e4ec5f5130fd1500f9a0

> 
> 
> Diff: https://reviews.apache.org/r/60821/diff/1/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>


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