> On May 11, 2017, 1:53 a.m., Jie Yu wrote: > > src/local/local.cpp > > Lines 372-374 (patched) > > <https://reviews.apache.org/r/58900/diff/5/?file=1714302#file1714302line372> > > > > 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.
This is logically a separate change (with some flag documentation updates), so I'll open a separate review with this change. - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58900/#review174618 ----------------------------------------------------------- On May 19, 2017, 3:05 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58900/ > ----------------------------------------------------------- > > (Updated May 19, 2017, 3:05 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 e172aa526cfd38f4f30efda22ef011146e5c1a7d > src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 > src/slave/slave.cpp 14de72fa4a8746504a251d735bf56041b934df40 > > > Diff: https://reviews.apache.org/r/58900/diff/6/ > > > Testing > ------- > > See last patch in chain. > > > Thanks, > > Joseph Wu > >
