> On Aug. 31, 2020, 9:14 p.m., Benjamin Mahler wrote:
> > src/master/master.cpp
> > Line 3291 (original), 3301 (patched)
> > <https://reviews.apache.org/r/72824/diff/1/?file=2238955#file2238955line3301>
> >
> > The PID could have changed, we can probably just remove that PID code
> > within `sendFrameworkUpdates` at this point.
> >
> > With the PID code still there, one can see that this if condition is
> > not capturing the potential PID change we'd want to tell the agent about.
>
> Andrei Sekretenko wrote:
> Hmm... I'm afraid I do not understand your point.
>
> First: can we really stop sending V0 framwork PID updates to agents?
> The comment around the framework pid in the messages sent by master to an
> agent doesn't say anything about the PID of the V0 frameworks, does it?
> Also, it looks like the agent code is using the framework PID for sending
> executor messages to the V0 frameworks bypassing the master.
> Although removing the PID update in `sendFrameworkUpdates()` doesn't
> break any tests, a naive attempt to remove the framework PID from the agent
> does.
> I haven't dived deep into that, but it looks like some tests depend on
> that bypass; my suspicion is that this V0 PID update is just not covered by
> tests.
>
> Second: why are you asking about the potential PID change in the
> `UpdateFramework` call?
> I'm looking at `Master::receive()`
> https://github.com/apache/mesos/blob/a16f3439dca13982bb4a2b9190c24aaf4eb73b0e/src/master/master.cpp#L2386
> and not getting how a V0 framework could change its PID without
> resubscribing...
Ah.. I see now from your comment and explanation, the PID cannot be changed in
this path. I also forgot that we do still use the PID based master bypass if
the scheduler is v0.
I guess eliminating the TODO is just the following:
```
// TODO(anand): We set 'pid' to UPID() for http frameworks
// as 'pid' was made optional in 0.24.0. In 0.25.0, we
// no longer have to set pid here for http frameworks.
message.set_pid(framework.pid().getOrElse(UPID()));
message.mutable_framework_info()->CopyFrom(framework.info);
send(slave->pid, message);
```
vs
```
*message.mutable_framework_info() = framework.info;
// Agents need to know about v0 framework's PIDs in order to
// bypass the master for executor to framework messages.
if (framework.pid().isSome()) {
message.set_pid(*framework.pid());
}
send(slave->pid, message);
```
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72824/#review221760
-----------------------------------------------------------
On Sept. 2, 2020, 2:08 p.m., Andrei Sekretenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72824/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2020, 2:08 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-10166
> https://issues.apache.org/jira/browse/MESOS-10166
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch makes the UPDATE_FRAMEWORK call avoid sending the
> `UpdatedFrameworkMessage` to agents and the `FRAMEWORK_UPDATED`
> event to V1 API subscribers when the FrameworkInfo is not changed
> by the call.
>
>
> Diffs
> -----
>
> src/master/master.cpp 02723296e569fac9d553b1494a5ca7daa6ef9aa4
>
>
> Diff: https://reviews.apache.org/r/72824/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Andrei Sekretenko
>
>
