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



src/tests/fetcher_cache_tests.cpp
<https://reviews.apache.org/r/30774/#comment140008>

    Any reason not using AWAIT_READY? CHECK_READY will abort the process while 
AWAIT_READY will just abort the test.
    
    ```
    19:53:22 DEBUG: F0611 19:53:23.743101 60646 fetcher_cache_tests.cpp:354] 
CHECK_READY(offers): is PENDING Failed to wait for resource offers
    19:53:22 DEBUG: *** Check failure stack trace: ***
    19:53:22 DEBUG:     @     0x7f431c3a644d  google::LogMessage::Fail()
    19:53:22 DEBUG:     @     0x7f431c3a828d  google::LogMessage::SendToLog()
    19:53:22 DEBUG:     @     0x7f431c3a603c  google::LogMessage::Flush()
    19:53:22 DEBUG:     @     0x7f431c3a8b89  
google::LogMessageFatal::~LogMessageFatal()
    19:53:22 DEBUG:     @           0x53d9b8  _CheckFatal::~_CheckFatal()
    19:53:22 DEBUG:     @           0x66c26f  
mesos::internal::tests::FetcherCacheTest::launchTask()
    19:53:22 DEBUG:     @           0x66fb09  
mesos::internal::tests::FetcherCacheTest_CachedFallback_Test::TestBody()
    19:53:22 DEBUG:     @           0xbb1db3  
testing::internal::HandleExceptionsInMethodIfSupported<>()
    19:53:22 DEBUG:     @           0xba9057  testing::Test::Run()
    19:53:22 DEBUG:     @           0xba90fe  testing::TestInfo::Run()
    19:53:22 DEBUG:     @           0xba9205  testing::TestCase::Run()
    19:53:22 DEBUG:     @           0xba94a8  
testing::internal::UnitTestImpl::RunAllTests()
    19:53:22 DEBUG:     @           0xba9747  testing::UnitTest::Run()
    19:53:22 DEBUG:     @           0x4a1dc3  main
    19:53:22 DEBUG:     @     0x7f431a1d7d5d  __libc_start_main
    19:53:22 DEBUG:     @           0x4ad109  (unknown)
    19:53:23 DEBUG: make[3]: *** [check-local] Aborted (core dumped)
    19:53:23 DEBUG: make[3]: Leaving directory 
`/builddir/build/BUILD/mesos-0.23.0/src'
    19:53:23 DEBUG: make[2]: *** [check-am] Error 2
    19:53:23 DEBUG: make[2]: Leaving directory 
`/builddir/build/BUILD/mesos-0.23.0/src'
    19:53:23 DEBUG: make[1]: *** [check] Error 2
    19:53:23 DEBUG: make[1]: Leaving directory 
`/builddir/build/BUILD/mesos-0.23.0/src'
    19:53:23 DEBUG: make: *** [check-recursive] Error 1
    19:53:23 DEBUG: RPM build errors:
    ```


- Jie Yu


On May 21, 2015, 4:05 p.m., Bernd Mathiske wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30774/
> -----------------------------------------------------------
> 
> (Updated May 21, 2015, 4:05 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Till Toenshoff, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-2057, MESOS-2069, MESOS-2070, MESOS-2072, MESOS-2073, and 
> MESOS-2074
>     https://issues.apache.org/jira/browse/MESOS-2057
>     https://issues.apache.org/jira/browse/MESOS-2069
>     https://issues.apache.org/jira/browse/MESOS-2070
>     https://issues.apache.org/jira/browse/MESOS-2072
>     https://issues.apache.org/jira/browse/MESOS-2073
>     https://issues.apache.org/jira/browse/MESOS-2074
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Almost all of the functionality in epic MESOS-336. Downloaded files from 
> CommandInfo::URIs can now be cached in a cache directory designated by a 
> slave flag. This only happens when asked for by an extra flag in the URI and 
> is thus backwards-compatible. The cache has a size limit also given by a new 
> slave flag. Cache-resident files are evicted as necessary to make space for 
> newly fetched ones. Concurrent attempts to cache the same URI leads to only 
> one download. The fetcher program remains external for safety reasons, but is 
> now augmented with more elaborate parameters packed into a JSON object to 
> implement specific fetch actions for all of the above. Additional testing 
> includes fetching from (mock) HDFS and coverage of the new features.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 54c4e31ed6dfed3c23d492c19a301ce119a0519b 
>   docs/fetcher-cache-internals.md PRE-CREATION 
>   docs/fetcher.md PRE-CREATION 
>   include/mesos/fetcher/fetcher.proto 
> 311af9aebc6a85dadba9dbeffcf7036b70896bcc 
>   include/mesos/mesos.proto 9cc5782256156ed59fd4640091413b76480d939f 
>   include/mesos/type_utils.hpp 837be6f1844d5fa01c0fd84a585e7ff2cc0c987b 
>   src/Makefile.am 34755cf795391c9b8051a5e4acc6caf844984496 
>   src/hdfs/hdfs.hpp 968545d9af896f3e72e156484cc58135405cef6b 
>   src/launcher/fetcher.cpp 796526f59c25898ef6db2b828b0e2bb7b172ba25 
>   src/slave/constants.hpp fd1c1aba0aa62372ab399bee5709ce81b8e92cec 
>   src/slave/containerizer/docker.hpp ed4ee19c85387882ab2e31baa5610acb8e222d50 
>   src/slave/containerizer/docker.cpp 408a4435a6f11973992486eac1659beeccc4beac 
>   src/slave/containerizer/fetcher.hpp 
> 1db0eaf002c8d0eaf4e0391858e61e0912b35829 
>   src/slave/containerizer/fetcher.cpp 
> 9e9e9d0eb6b0801d53dec3baea32a4cd4acdd5e2 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3e18617b0dbac58176bfd41dc550898eb6a4a79e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 696e359de66305512eedf8e269543fafa21f4bc3 
>   src/slave/flags.hpp 5c57478fcfdbcbd8ac0e5c3c79809403054e96e6 
>   src/slave/flags.cpp b5e25186dad36bc1306cc6ecb268aba951a18f7e 
>   src/slave/slave.cpp 8e88482f41f37ce7f2559fe793565b66ac46fb35 
>   src/tests/docker_containerizer_tests.cpp 
> 154bf981c007ebcb8e0b2fe8551defb5ea2ba063 
>   src/tests/fetcher_cache_tests.cpp PRE-CREATION 
>   src/tests/fetcher_tests.cpp 4549e6a631e2c17cec3766efaa556593eeac9a1e 
>   src/tests/mesos.hpp a60df75350beab8d7091cbe66213ecd920942fa4 
>   src/tests/mesos.cpp 1d5639c85517229f3396b40f2d8bd421b2ed7325 
> 
> Diff: https://reviews.apache.org/r/30774/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> --- longer Description: ---
> 
> -Replaces all other reviews for the fetcher cache except those related to 
> stout: 30006, 30033, 30034, 30036, 30037, 30039, 30124, 30173, 30614, 30616, 
> 30618, 30621, 30626. See descriptions of those. In dependency order:
> 
> 30033: Removes the fetcher env tests since these won't be needed any more 
> when the fetcher uses JSON in a single env var as a parameter. They never 
> tested anything that won't be covered by other tests anyway.
> 
> 30034: Makes the code structure of all fetcher tests the same. Instead of 
> calling the run method of the fetcher directly, calling through fetch(). Also 
> removes all uses of I/O redirection, which is not really needed for 
> debugging, and thus the next patch can refactor fetch() and run(). (The 
> latter comes in two varieties, which complicates matters without much 
> benefit.)
> 
> 30036: Extends the CommandInfo::URI protobuf with a boolean "caching" field 
> that will later cause fetcher cache actions. Also introduces the notion of a 
> cache directory to the fetcher info protobuf. And then propagates these 
> additions throughout the rest of the code base where applicable. This 
> includes passing the slave ID all the way down to the place where the cache 
> dir name is constructed.
> 
> 30037: Extends the fetcher info protobuf with "actions" (fetch directly 
> bypassing the cache, fetch through the cache, retrieve from the cache). 
> Switches the basis for dealing with uris to "items", which contain the uri, 
> the action, and potentially a cache file name. Refactors fetch() and run(), 
> so there is only one of each. Introduces about half of the actual cache 
> logic, including a hashmap of cache file objects for bookkeeping and basic 
> operations on it. 
> 
> 30039: Enables fetcher cache actions in the mesos fetcher program.
> 
> 30006: Enables concurrent downloading into the fetcher cache. Reuse of 
> download results in the cache when multiple fetcher runs occur concurrently. 
> 
> 30614: This is to ensure that all this refactoring of fetcher code has not 
> broken HDFS fetching. Adds a test that exercises the C++ code paths in Mesos 
> and mesos-fetcher related to fetching from HDFS. Uses a mock HDFS client 
> written in bash that acts just like a real "hadoop" command if used in the 
> right limited way.
> 
> 30124: Inserted fetcher cache zap upon slave startup, recovery and shutdown. 
> This implements recovery in an acceptable, yet most simple way.
> 
> 30173: Created fetcher cache tests. Adds a new test source file containing a 
> test fixture and tests to find out if the fetcher cache works with a variety 
> of settings.
> 
> 30616: Adds hdfs::du() which calls "hadoop fs -du -h" and returns a string 
> that contains the file size for the URI passed as argument. This is needed to 
> determine the size of a file on HDFS before downloading it to the fetcher 
> cache (to ensure there is enough space).
> 
> 30621: Refactored URI type separation in mesos-fetcher. Moved the URI type 
> separation code (distinguishes http, hdfs, local copying, etc.) from 
> mesos-fetcher to the fetcher process/actor, since it is going to be reused by 
> download size queries when we introduce fetcher cache management. Also 
> factored out URI validation, which will be used the same way by mesos-fetcher 
> and the fetcher process/actor.
> 
> 30626: Fetcher cache eviction. This happens when the cache does not have 
> enough space to accomodate upcoming downloads to the cache. Necessary 
> provisions included here:
> - mesos-fetcher does not run until evictions have been successful
> - Cache space is reserved while (async) waiting for eviction to succeed. If 
> it fails, the reservation gets undone.
> - Reservations can be partly from available space, partly from evictions. All 
> math included :-)
> - To find out how much space is needed, downloading has a prelude in which we 
> query the download size from the URI. This works for all URI types that 
> mesos-fetcher currently supports, including http and hdfs.
> - Size-determination requests are now synchronized, too. Only one per URI in 
> play happens.
> - There is cleanup code for all kinds of error situations. At the very end of 
> the fetch attempt, each list is processed for undoing things like space 
> reservations and eviction disabling.
> - Eviction gets disabled for URIs that are currently in use, i.e. the related 
> cache files are. We use reference counting for this, since there may be 
> concurrent fetch attempts using the same cache files.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Reply via email to