> On March 7, 2018, 4:13 a.m., Greg Mann wrote: > > src/master/master.cpp > > Line 7722 (original), 7680-7681 (patched) > > <https://reviews.apache.org/r/65591/diff/7/?file=1971488#file1971488line7847> > > > > Perhaps we also want to rescind offers when > > `message.has_resource_providers()` is false? In that case, the agent has > > told us that it has no registered RPs, so offers with RP resources would > > not be valid? > > Benjamin Bannier wrote: > I do not think that is true. Only when the `resource_providers` field is > set does the agent send any resource provider information. The field could > contain information on zero or more resource providers in its `providers` > field, but we do not distinguish that here, exactly as you suggest. > > Dropping. > > Greg Mann wrote: > IIUC, the agent always sends info on all registered RPs in each > UpdateSlaveMessage. So, if the master receives an UpdateSlaveMessage which > indicates that no RPs are currently registered on an agent, doesn't that mean > that offers for that agent with RP resources are currently invalid?
Yes, the offers will be rescinded below. > On March 7, 2018, 4:13 a.m., Greg Mann wrote: > > src/master/master.cpp > > Lines 10572-10574 (original), 10534-10536 (patched) > > <https://reviews.apache.org/r/65591/diff/7/?file=1971488#file1971488line10701> > > > > Are you sure this will always be true? Could a framework send us a > > well-formed, but unknown RP ID in a message and make this false? > > Benjamin Bannier wrote: > This is an important invariant of the current implementation, see > https://issues.apache.org/jira/browse/MESOS-8321. > > Dropping. > > Greg Mann wrote: > Are we sure that invariant holds in this case? The resource provider ID > here is framework-supplied, and I don't think we validate that it's present > in the master state before we hit this code. On the accept path the master will verify that (i) it knows about the offer (https://github.com/apache/mesos/blob/843e5e85939d848b0898753c9d7542ecc997135c/src/master/master.cpp#L3906-L3924), and (ii) that the operations are consistent with the originally offered resources (https://github.com/apache/mesos/blob/843e5e85939d848b0898753c9d7542ecc997135c/src/master/master.cpp#L4530-L4544). For a resource provider to be unknown to the master here it would either have to be explicitly removed by the master, or be made up by the framework. In the first case the offer would be invalid, in the second the operation consistency check would fail. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65591/#review198741 ----------------------------------------------------------- On March 7, 2018, 12:36 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65591/ > ----------------------------------------------------------- > > (Updated March 7, 2018, 12:36 p.m.) > > > Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht. > > > Bugs: MESOS-8382 > https://issues.apache.org/jira/browse/MESOS-8382 > > > Repository: mesos > > > Description > ------- > > This patch adds explicit tracking of resource providers to the master > process. While we already had explicitly send resource provider > information in e.g., `UpdateSlaveMessage`, we only stored that > information aggregated over the full agent in the master up to now. > > > Diffs > ----- > > src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 > src/master/http.cpp acf10a0f396234ec9f444c9b41515d657379d6ad > src/master/master.hpp c9c8a968b6f56fe261ac9fe374e926a28d40fccb > src/master/master.cpp e666b664dd125317cda5d16285d444b9c21e1f35 > > > Diff: https://reviews.apache.org/r/65591/diff/8/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
