You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2017/12/01 01:05:56 UTC

Review Request 64238: Updated the allocator to track allocations via a single code path.

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

Review request for mesos, Benjamin Bannier, Jie Yu, Michael Park, and Meng Zhu.


Repository: mesos


Description
-------

A helper was introduced for tracking allocated resources in the
sorters. This updates the code to allow the other two copies of
this code to use the function.

This also documents some of the addFramework/addSlave cases.


Diffs
-----

  src/master/allocator/mesos/hierarchical.hpp 3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
  src/master/allocator/mesos/hierarchical.cpp ab2abf868f9252154d934243521622c5cb107182 


Diff: https://reviews.apache.org/r/64238/diff/1/


Testing
-------

make check


Thanks,

Benjamin Mahler


Re: Review Request 64238: Updated the allocator to track allocations via a single code path.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64238/#review192483
-----------------------------------------------------------



FAIL: Mesos libprocess-tests failed to build

Reviews applied: `['64236', '64237', '64238']`

Failed command: `cmake.exe --build . --target libprocess-tests --config Debug`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64238

Relevant logs:

- [libprocess-tests-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64238/logs/libprocess-tests-build-cmake-stdout.log):

```
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp)system_tests.cpp [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of data (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\state_machine_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of data (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\system_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of data (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\subprocess_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(805): warning C4244: 'argument': conversion from 'ULONG_PTR' to 'pid_t', possible loss of data (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\time_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(62): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\stout\include\stout/windows/os.hpp(451): warning C4996: 'GetVersionExW': was declared deprecated (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]


"C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" (default target) (1) ->
(ClCompile target) -> 
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp(355): error C2039: 'realpath': is not a member of 'os' (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\http_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp(355): error C3861: 'realpath': identifier not found (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\http_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp(355): error C2039: 'realpath': is not a member of 'os' (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\socket_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp(355): error C3861: 'realpath': identifier not found (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\socket_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp(355): error C2039: 'realpath': is not a member of 'os' (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/ssl/gtest.hpp(355): error C3861: 'realpath': identifier not found (compiling source file C:\DCOS\mesos\mesos\3rdparty\libprocess\src\tests\ssl_tests.cpp) [C:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj]

    96 Warning(s)
    6 Error(s)

Time Elapsed 00:04:26.92
```

- [libprocess-tests-CMakeOutput.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64238/logs/libprocess-tests-CMakeOutput.log):

```
  Creating directory "C:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\".

  Creating directory "cmTC_80e84.dir\Debug\cmTC_80e84.tlog\".

InitializeBuildStatus:

  Creating "cmTC_80e84.dir\Debug\cmTC_80e84.tlog\unsuccessfulbuild" because "AlwaysCreate" was specified.

ClCompile:

  C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.25017\bin\HostX64\x64\CL.exe /c /Zi /W3 /WX- /diagnostics:classic /Od /Ob0 /D WIN32 /D _WINDOWS /D COMPILER_SUPPORTS_CXX11 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /EHsc /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /Fo"cmTC_80e84.dir\Debug\" /Fd"cmTC_80e84.dir\Debug\vc141.pdb" /Gd /TP /errorReport:queue C:\DCOS\mesos\CMakeFiles\CMakeTmp\src.cxx

  Microsoft (R) C/C++ Optimizing Compiler Version 19.10.25019 for x64

  Copyright (C) Microsoft Corporation.  All rights reserved.

  

  cl /c /Zi /W3 /WX- /diagnostics:classic /Od /Ob0 /D WIN32 /D _WINDOWS /D COMPILER_SUPPORTS_CXX11 /D "CMAKE_INTDIR=\"Debug\"" /D _MBCS /Gm- /EHsc /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /Fo"cmTC_80e84.dir\Debug\" /Fd"cmTC_80e84.dir\Debug\vc141.pdb" /Gd /TP /errorReport:queue C:\DCOS\mesos\CMakeFiles\CMakeTmp\src.cxx

  src.cxx

  

Link:

  C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.25017\bin\HostX64\x64\link.exe /ERRORREPORT:QUEUE /OUT:"C:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\cmTC_80e84.exe" /INCREMENTAL /NOLOGO kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTUAC:"level='asInvoker' uiAccess='false'" /manifest:embed /DEBUG /PDB:"C:/DCOS/mesos/CMakeFiles/CMakeTmp/Debug/cmTC_80e84.pdb" /SUBSYSTEM:CONSOLE /TLBID:1 /DYNAMICBASE /NXCOMPAT /IMPLIB:"C:/DCOS/mesos/CMakeFiles/CMakeTmp/Debug/cmTC_80e84.lib" /MACHINE:X64  /machine:x64 cmTC_80e84.dir\Debug\src.obj

  cmTC_80e84.vcxproj -> C:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\cmTC_80e84.exe

  cmTC_80e84.vcxproj -> C:/DCOS/mesos/CMakeFiles/CMakeTmp/Debug/cmTC_80e84.pdb (Full PDB)

FinalizeBuildStatus:

  Deleting file "cmTC_80e84.dir\Debug\cmTC_80e84.tlog\unsuccessfulbuild".

  Touching "cmTC_80e84.dir\Debug\cmTC_80e84.tlog\cmTC_80e84.lastbuildstate".

Done Building Project "C:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_80e84.vcxproj" (default targets).



Build succeeded.

    0 Warning(s)

    0 Error(s)



Time Elapsed 00:00:01.78


Source file was:
int main() { return 0; }
```

- [libprocess-tests-CMakeError.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/64238/logs/libprocess-tests-CMakeError.log):

```
PrepareForBuild:

  Creating directory "cmTC_8ba6f.dir\Debug\".

  Creating directory "C:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\".

  Creating directory "cmTC_8ba6f.dir\Debug\cmTC_8ba6f.tlog\".

InitializeBuildStatus:

  Creating "cmTC_8ba6f.dir\Debug\cmTC_8ba6f.tlog\unsuccessfulbuild" because "AlwaysCreate" was specified.

ClCompile:

  C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.25017\bin\HostX64\x64\CL.exe /c /Zi /W3 /WX- /diagnostics:classic /MP /Od /Ob0 /D WIN32 /D _WINDOWS /D UNICODE /D _UNICODE /D "CMAKE_INTDIR=\"Debug\"" /D _UNICODE /D UNICODE /Gm- /RTC1 /MTd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /Fo"cmTC_8ba6f.dir\Debug\" /Fd"cmTC_8ba6f.dir\Debug\vc141.pdb" /Gd /TC /errorReport:queue C:\DCOS\mesos\CMakeFiles\CMakeTmp\CheckIncludeFile.c

  Microsoft (R) C/C++ Optimizing Compiler Version 19.10.25019 for x64

  Copyright (C) Microsoft Corporation.  All rights reserved.

  

  cl /c /Zi /W3 /WX- /diagnostics:classic /MP /Od /Ob0 /D WIN32 /D _WINDOWS /D UNICODE /D _UNICODE /D "CMAKE_INTDIR=\"Debug\"" /D _UNICODE /D UNICODE /Gm- /RTC1 /MTd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /Fo"cmTC_8ba6f.dir\Debug\" /Fd"cmTC_8ba6f.dir\Debug\vc141.pdb" /Gd /TC /errorReport:queue C:\DCOS\mesos\CMakeFiles\CMakeTmp\CheckIncludeFile.c

  CheckIncludeFile.c

  

C:\DCOS\mesos\CMakeFiles\CMakeTmp\CheckIncludeFile.c(1): fatal error C1083: Cannot open include file: 'pthread.h': No such file or directory [C:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_8ba6f.vcxproj]

Done Building Project "C:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_8ba6f.vcxproj" (default targets) -- FAILED.



Build FAILED.



"C:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_8ba6f.vcxproj" (default target) (1) ->

(ClCompile target) -> 

  C:\DCOS\mesos\CMakeFiles\CMakeTmp\CheckIncludeFile.c(1): fatal error C1083: Cannot open include file: 'pthread.h': No such file or directory [C:\DCOS\mesos\CMakeFiles\CMakeTmp\cmTC_8ba6f.vcxproj]



    0 Warning(s)

    1 Error(s)



Time Elapsed 00:00:01.11



```

- Mesos Reviewbot Windows


On Dec. 1, 2017, 1:05 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64238/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2017, 1:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Michael Park, and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A helper was introduced for tracking allocated resources in the
> sorters. This updates the code to allow the other two copies of
> this code to use the function.
> 
> This also documents some of the addFramework/addSlave cases.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
>   src/master/allocator/mesos/hierarchical.cpp ab2abf868f9252154d934243521622c5cb107182 
> 
> 
> Diff: https://reviews.apache.org/r/64238/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 64238: Updated the allocator to track allocations via a single code path.

Posted by Benjamin Mahler <bm...@apache.org>.

> On Dec. 4, 2017, 3:20 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 676 (patched)
> > <https://reviews.apache.org/r/64238/diff/1/?file=1905334#file1905334line683>
> >
> >     We will only call `addResourceProvider` after `slaveId` has been added to the allocator (`addResourceProvider` is triggered by `UpdateSlaveMessage` which will only be sent after registration).
> >     
> >     We still might run into case (2) here where there are pending offer operations by some framework on the added resource provider and we never ran any tasks from that framework on that agent so the `FrameworkInfo` is unknown to the agent.
> 
> Benjamin Mahler wrote:
>     That sounds like a bug in the new provider logic? Shouldn't the agent be sending the framework info in that case?
> 
> Benjamin Bannier wrote:
>     My comment above is incorrect. I missed that we broadcast any new framework's `FrameworkInfo` on framework subscription; in that case we would always send the agent the `FrameworkInfo` before sending operations from that framework, and consequentially any `UpdateSlaveMessage` sent during agent resource and offer operation reconciliation after a master failover should only contain operations from frameworks already known to the new master from `ReregisterSlaveMessage`.

Ok, I won't CHECK for it for now (up to you if you want to do a more careful reading of the code and CHECK it) but I will say it shouldn't be possible.


- Benjamin


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


On Dec. 1, 2017, 1:05 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64238/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2017, 1:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Michael Park, and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A helper was introduced for tracking allocated resources in the
> sorters. This updates the code to allow the other two copies of
> this code to use the function.
> 
> This also documents some of the addFramework/addSlave cases.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
>   src/master/allocator/mesos/hierarchical.cpp ab2abf868f9252154d934243521622c5cb107182 
> 
> 
> Diff: https://reviews.apache.org/r/64238/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 64238: Updated the allocator to track allocations via a single code path.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On Dec. 4, 2017, 4:20 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 676 (patched)
> > <https://reviews.apache.org/r/64238/diff/1/?file=1905334#file1905334line683>
> >
> >     We will only call `addResourceProvider` after `slaveId` has been added to the allocator (`addResourceProvider` is triggered by `UpdateSlaveMessage` which will only be sent after registration).
> >     
> >     We still might run into case (2) here where there are pending offer operations by some framework on the added resource provider and we never ran any tasks from that framework on that agent so the `FrameworkInfo` is unknown to the agent.
> 
> Benjamin Mahler wrote:
>     That sounds like a bug in the new provider logic? Shouldn't the agent be sending the framework info in that case?

My comment above is incorrect. I missed that we broadcast any new framework's `FrameworkInfo` on framework subscription; in that case we would always send the agent the `FrameworkInfo` before sending operations from that framework, and consequentially any `UpdateSlaveMessage` sent during agent resource and offer operation reconciliation after a master failover should only contain operations from frameworks already known to the new master from `ReregisterSlaveMessage`.


- Benjamin


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


On Dec. 1, 2017, 2:05 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64238/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2017, 2:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Michael Park, and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A helper was introduced for tracking allocated resources in the
> sorters. This updates the code to allow the other two copies of
> this code to use the function.
> 
> This also documents some of the addFramework/addSlave cases.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
>   src/master/allocator/mesos/hierarchical.cpp ab2abf868f9252154d934243521622c5cb107182 
> 
> 
> Diff: https://reviews.apache.org/r/64238/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 64238: Updated the allocator to track allocations via a single code path.

Posted by Benjamin Mahler <bm...@apache.org>.

> On Dec. 4, 2017, 3:20 p.m., Benjamin Bannier wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 676 (patched)
> > <https://reviews.apache.org/r/64238/diff/1/?file=1905334#file1905334line683>
> >
> >     We will only call `addResourceProvider` after `slaveId` has been added to the allocator (`addResourceProvider` is triggered by `UpdateSlaveMessage` which will only be sent after registration).
> >     
> >     We still might run into case (2) here where there are pending offer operations by some framework on the added resource provider and we never ran any tasks from that framework on that agent so the `FrameworkInfo` is unknown to the agent.

That sounds like a bug in the new provider logic? Shouldn't the agent be sending the framework info in that case?


- Benjamin


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


On Dec. 1, 2017, 1:05 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64238/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2017, 1:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Michael Park, and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A helper was introduced for tracking allocated resources in the
> sorters. This updates the code to allow the other two copies of
> this code to use the function.
> 
> This also documents some of the addFramework/addSlave cases.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
>   src/master/allocator/mesos/hierarchical.cpp ab2abf868f9252154d934243521622c5cb107182 
> 
> 
> Diff: https://reviews.apache.org/r/64238/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


Re: Review Request 64238: Updated the allocator to track allocations via a single code path.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64238/#review192695
-----------------------------------------------------------


Fix it, then Ship it!




Thanks for the cleanup Ben!


src/master/allocator/mesos/hierarchical.cpp
Lines 549 (patched)
<https://reviews.apache.org/r/64238/#comment270953>

    Let's be less alarmist and  `s/!/.`.



src/master/allocator/mesos/hierarchical.cpp
Lines 674 (patched)
<https://reviews.apache.org/r/64238/#comment270952>

    Let's be less alarmist and  `s/!/.`.



src/master/allocator/mesos/hierarchical.cpp
Lines 676 (patched)
<https://reviews.apache.org/r/64238/#comment270951>

    We will only call `addResourceProvider` after `slaveId` has been added to the allocator (`addResourceProvider` is triggered by `UpdateSlaveMessage` which will only be sent after registration).
    
    We still might run into case (2) here where there are pending offer operations by some framework on the added resource provider and we never ran any tasks from that framework on that agent so the `FrameworkInfo` is unknown to the agent.


- Benjamin Bannier


On Dec. 1, 2017, 2:05 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64238/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2017, 2:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, Michael Park, and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> A helper was introduced for tracking allocated resources in the
> sorters. This updates the code to allow the other two copies of
> this code to use the function.
> 
> This also documents some of the addFramework/addSlave cases.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 3c87dc797cf70f3aa48b1ed9f86d673d4ea2fe76 
>   src/master/allocator/mesos/hierarchical.cpp ab2abf868f9252154d934243521622c5cb107182 
> 
> 
> Diff: https://reviews.apache.org/r/64238/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>