mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil Conway <neil.con...@gmail.com>
Subject Re: Review Request 60407: Avoided master crash on agent re-registration.
Date Sat, 24 Jun 2017 19:30:00 GMT


> On June 24, 2017, 2:35 a.m., Adam B wrote:
> > src/master/validation.cpp
> > Line 370 (original), 370-372 (patched)
> > <https://reviews.apache.org/r/60407/diff/2/?file=1761693#file1761693line370>
> >
> >     Even if we can't `validateDynamicReservationInfo` (or  `validateDiskInfo`?),
would it be worthwhile to `validateGpus`? Maybe clone/parameterize `resource::validate` to
validate what we can?

We could certainly do better, but since all this validation is best-effort anyway, I think
the current approach is okay for now. I added `TODO`s to note this for future improvement.


- Neil


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


On June 24, 2017, 7:28 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60407/
> -----------------------------------------------------------
> 
> (Updated June 24, 2017, 7:28 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When validating the agent's ReregisterSlaveMessage, the master's
> validation code neglected to account for the fact that the task
> resources might not be in post-refinement format (e.g., if the agent
> does not support reservation refinement). This lead to a `CHECK` failure
> during validation.
> 
> Fix this by relaxing the validation of ReregisterSlaveMessage so that we
> do not depend on the task resources being in post-refinement
> format. This means validation of ReregisterSlaveMessage will be less
> effective, but since it is best-effort anyway, this seems tolerable.
> 
> 
> Diffs
> -----
> 
>   src/master/validation.cpp 33e9ff7db9e2789cbb2d6dfd015288dfa1faa7c5 
> 
> 
> Diff: https://reviews.apache.org/r/60407/diff/3/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


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