----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66644/#review201750 -----------------------------------------------------------
src/master/master.cpp Lines 7130 (patched) <https://reviews.apache.org/r/66644/#comment283338> I'd add a comment here explaining what we are doing. Maybe something like: "All tasks from this agent are now reachable. Update framework bookkeeping, removing tasks that were dropped en-route to the agent and marked as unreachable." src/tests/partition_tests.cpp Lines 167-168 (patched) <https://reviews.apache.org/r/66644/#comment283324> I'd write something like: ``` This is a regression test for MESOS-8750. It verifies that a master won't crash when a `RunTaskMessage` associated to a partition aware framework is dropped en route to the agent and then the agent fails over. ``` src/tests/partition_tests.cpp Lines 169 (patched) <https://reviews.apache.org/r/66644/#comment283308> s/cleanUpUnreachableTasks/CleanUpUnreachableTasks/ src/tests/partition_tests.cpp Lines 182 (patched) <https://reviews.apache.org/r/66644/#comment283309> The indentation here looks off. src/tests/partition_tests.cpp Lines 200 (patched) <https://reviews.apache.org/r/66644/#comment283312> Ditto indentation. src/tests/partition_tests.cpp Lines 206-207 (patched) <https://reviews.apache.org/r/66644/#comment283313> Ditto indentation. src/tests/partition_tests.cpp Lines 216 (patched) <https://reviews.apache.org/r/66644/#comment283317> s/Offer/const Offer&/ src/tests/partition_tests.cpp Lines 220 (patched) <https://reviews.apache.org/r/66644/#comment283320> s/Capture/Drop/ ? Where is the pid unset? I think I'd write something like: "Drop the `RunTaskMessage`", so that the agent doesn't learn about the new task. src/tests/partition_tests.cpp Lines 222 (patched) <https://reviews.apache.org/r/66644/#comment283318> Ditto indentation. src/tests/partition_tests.cpp Lines 228 (patched) <https://reviews.apache.org/r/66644/#comment283319> Remove this blank line. src/tests/partition_tests.cpp Lines 235 (patched) <https://reviews.apache.org/r/66644/#comment283323> Ditto indentation. src/tests/partition_tests.cpp Lines 241 (patched) <https://reviews.apache.org/r/66644/#comment283325> Ditto indentation. src/tests/partition_tests.cpp Lines 279-282 (patched) <https://reviews.apache.org/r/66644/#comment283326> Ditto indentation. src/tests/partition_tests.cpp Lines 303 (patched) <https://reviews.apache.org/r/66644/#comment283327> Ditto indentation. src/tests/partition_tests.cpp Lines 317 (patched) <https://reviews.apache.org/r/66644/#comment283328> Ditto indentation. src/tests/partition_tests.cpp Lines 319-323 (patched) <https://reviews.apache.org/r/66644/#comment283329> Ditto indentation. src/tests/partition_tests.cpp Lines 328-332 (patched) <https://reviews.apache.org/r/66644/#comment283330> Ditto indentation. src/tests/partition_tests.cpp Lines 346-350 (patched) <https://reviews.apache.org/r/66644/#comment283331> Ditto indentation. src/tests/partition_tests.cpp Lines 355-359 (patched) <https://reviews.apache.org/r/66644/#comment283332> Ditto indentation. src/tests/partition_tests.cpp Lines 379 (patched) <https://reviews.apache.org/r/66644/#comment283336> s/reregister/re-register/ src/tests/partition_tests.cpp Lines 384 (patched) <https://reviews.apache.org/r/66644/#comment283333> Ditto indentation. src/tests/partition_tests.cpp Lines 433-437 (patched) <https://reviews.apache.org/r/66644/#comment283334> Ditto indentation. src/tests/partition_tests.cpp Lines 452-456 (patched) <https://reviews.apache.org/r/66644/#comment283335> Ditto indentation. - Gaston Kleiman On April 23, 2018, 11:11 a.m., Megha Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66644/ > ----------------------------------------------------------- > > (Updated April 23, 2018, 11:11 a.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: 8750 > https://issues.apache.org/jira/browse/8750 > > > Repository: mesos > > > Description > ------- > > A RunTask messsage could get dropped for an agent while it's > disconnected from the master and when such an agent goes unreachable > then this dropped task message gets added to the unreachable tasks. > When the agent re-registers, the master sends status updates for the > tasks that the agent reported when re-registering and these tasks are > also removed from the unreachableTasks on the framework but since the > agent doesn't know about the dropped task so it doesn't get removed > from the unreachableTasks leading to a check failure when > this inconsistency is detected during framework removal. > > > Diffs > ----- > > src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 > src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a > src/tests/partition_tests.cpp 9138e5c745cf354a3573e1ab0b251d46702833cc > > > Diff: https://reviews.apache.org/r/66644/diff/1/ > > > Testing > ------- > > > Thanks, > > Megha Sharma > >
