----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58900/#review174618 -----------------------------------------------------------
Great clean up! src/local/local.cpp Lines 372-374 (patched) <https://reviews.apache.org/r/58900/#comment247822> I'd suggest we put fetcher dir under work_dir as well for local mode so that all directories related to this local run have a common parent directory. ``` propagatedFlags["work_dir"] = path::join(flags.work_dir, "agents", stringify(i), "work_dir"); propagatedFlags["runtime_dir"] = path::join(flags.work_dir, "agents", stringify(i), "runtime_dir"); propagatedFlags["fetcher_cache_dir"] = path::join(flags.work_dir, "agents", stringify(i), "fetcher_cache_dir"); ``` No need for the `runtime_dir` in local flags. - Jie Yu On May 10, 2017, 8:32 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58900/ > ----------------------------------------------------------- > > (Updated May 10, 2017, 8:32 p.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-7304 > https://issues.apache.org/jira/browse/MESOS-7304 > > > Repository: mesos > > > Description > ------- > > The fetcher cache directory was historically located (by default) > in `/tmp/mesos/fetch`. The agent flag `--fetcher_cache_dir` could > be used to change this value. > > The fetcher would create a subdirectory underneath `/tmp/mesos/fetch` > for each `SlaveID`. This was done because multiple agents can run on > the same node. If all the agents use the same default fetcher cache > directory, they will collide and cause unpredictable results. > As a result, the `SlaveID` needed to be passed into the fetcher > after the agent recovers and/or registers with the master, because > that is when the `SlaveID` is determined. > > This changes the default fetcher cache directory to > `/tmp/mesos/fetch`. The `SlaveID` subdirectory has been removed. > > This change, while techically a breaking change, is safe because of > how the fetcher uses this directory. Upon starting up, the fetcher > "recovers" by clearing this directory. Although the subdirectory > has been removed, the fetcher still clears the fetcher cache > on startup. > > This change will only cause breakages if multiple agents are run > with the same `--fetcher_cache_dir`. In this case, each agent > will delete the fetcher caches of all the other agents. > > --- > > With the removal of the `SlaveID` field in the fetcher's methods, > it is no longer necessary to pass in the `SlaveID` or agent Flags > at agent recovery time. Instead, the flags can be passed in during > the fetcher's construction. > > Similarly, the fetcher's "recovery" (clearing the fetcher cache) > can be done immediately upon construction, which simplifies the > code slightly. > > > Diffs > ----- > > src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 > src/slave/containerizer/fetcher.hpp > 9e3018dc087ed55c61b2824d0105bc5339b83043 > src/slave/containerizer/fetcher.cpp > a910fea5a5556afb376524c5bb2ff98d7d84e611 > src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c > src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 > src/slave/slave.cpp 209aff180db1a68ae360d7c278937fe645efdb2b > > > Diff: https://reviews.apache.org/r/58900/diff/5/ > > > Testing > ------- > > See last patch in chain. > > > Thanks, > > Joseph Wu > >
