> On Dec. 1, 2017, 9:55 p.m., Ilya Pronin wrote: > > docs/task-state-reasons.md > > Lines 474-477 (patched) > > <https://reviews.apache.org/r/64250/diff/2/?file=1906002#file1906002line474> > > > > I don't quite follow this note. A modified copy of which update? Should > > we just say that these updates reflect the states of the tasks reported by > > the agent upon its re-registration? > > Megha Sharma wrote: > Here, we are saying that master takes the actual state reported by the > agent and enriches it with reason and message before sending to the > framework. But I am open to changing it if you or Yan feel it doesn't convery > the idea. > > Jiang Yan Xu wrote: > I was suggesting it because this status is generated by the master the > same way they are generated upon reconciliation. > > For `REASON_RECONCILIATION` in the same doc there is this note: > > ``` > Note: Status updates with this reason are not the original ones, but > rather a modified copy that is re-sent from the master. In particular, the > original data and message fields are erased and the original reason field is > overwritten by REASON_RECONCILIATION . > ``` > > It is the same for `REASON_SLAVE_REREGISTERED`. > > > Megha I just noticed that you didn't mention the erasure of `data` and > `message` fields, but perhaps this is not worth repeating. I was originally > suggesting refering to `REASON_RECONCILIATION` for details: something like: > `Status updates with this reason are a modified copies re-sent by the master. > See comments for REASON_RECONCILIATION.` > > Would this be clearer? > > "reflect the states of the tasks reported by the agent upon its > re-registration" this sentence is good too. > > Ilya Pronin wrote: > Oh, now I got where this came from :) Yeah, it would be clearer if there > was a mention that new states come from the agent re-registration message and > a reference to `REASON_RECONCILIATION` description. Or it should be enough to > jsut say that these are produced by the master and reflect the states > reported by the agent during re-registration.
PTAL, updated the description based on the discussion. - Megha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64250/#review192567 ----------------------------------------------------------- On Dec. 1, 2017, 9:06 p.m., Megha Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64250/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2017, 9:06 p.m.) > > > Review request for mesos, Ilya Pronin, James Peach, and Jiang Yan Xu. > > > Repository: mesos > > > Description > ------- > > Added new reasons `REASON_SLAVE_REREGISTERED` and > `REASON_AGENT_REREGISTERED` for v0 and v1 task status update. > The new reason will be used when master starts to send status > update during the re-registration of an agent which was > previously removed by the master because of being unreachable > or is unknown to the master due to the garbage collection of > the unreachable and gone agents in the registry. > > > Diffs > ----- > > docs/task-state-reasons.md 07f7927e28fe5624a6c9b74de731483aaa729ca3 > include/mesos/mesos.proto 11089b7d6f48f001ae64e8b0c67a2732684aa2fa > include/mesos/v1/mesos.proto c496da927903f472c78d455b7fdf886d8fb692c4 > > > Diff: https://reviews.apache.org/r/64250/diff/5/ > > > Testing > ------- > > code changes verified with make check and the documents changes with markdown > viewer > > > Thanks, > > Megha Sharma > >