Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/14871 )
Change subject: fs: separate out directory management code
......................................................................
Patch Set 6:
(3 comments)
Looks like a lot of code motion. Anything of substance changed in the existing
methods that were moved from DataDir{Manager} to Dir{Manager}?
http://gerrit.cloudera.org:8080/#/c/14871/6/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/14871/6/src/kudu/fs/data_dirs.cc@220
PS6, Line 220: : DirManager(env, opts.metric_entity ?
: unique_ptr<DirMetrics>(new
DataDirMetrics(opts.metric_entity)) : nullptr,
: FLAGS_fs_max_thread_count_per_data_dir,
: opts, std::move(canonicalized_data_roots)) {}
Is it unsafe to pass opts by move because of these read accesses? What are the
rules on that?
http://gerrit.cloudera.org:8080/#/c/14871/6/src/kudu/fs/data_dirs.cc@327
PS6, Line 327: // Otherwise, use the default UUID assignment.
: return DirManager::PopulateDirectoryMaps(dirs);
Maybe invert with the above for clarity (i.e. reduced nesting)?
http://gerrit.cloudera.org:8080/#/c/14871/6/src/kudu/fs/fs_manager.cc
File src/kudu/fs/fs_manager.cc:
http://gerrit.cloudera.org:8080/#/c/14871/6/src/kudu/fs/fs_manager.cc@a411
PS6, Line 411:
Not sure why we aren't setting this anymore?
--
To view, visit http://gerrit.cloudera.org:8080/14871
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I633e1e32845b08eb24c5327a04af344b579b186a
Gerrit-Change-Number: 14871
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 14 Dec 2019 01:12:01 +0000
Gerrit-HasComments: Yes
