hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anatoli Shein (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-10754) libhdfs++: Create tools directory and implement hdfs_cat, hdfs_chgrp, hdfs_chown, hdfs_chmod and hdfs_find.
Date Wed, 24 Aug 2016 21:13:20 GMT

    [ https://issues.apache.org/jira/browse/HDFS-10754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15435697#comment-15435697

Anatoli Shein commented on HDFS-10754:

Thanks for the review, [~bobhansen].

I have addressed your comments as follows:
* Why are we including "google/protobuf/stubs/common.h?
(i) Because we need it to call ShutdownProtobufLibrary() (clean up for protobuf).
* Comment says "wrapping fs in unique_ptr", but then we wrap it in a shared_ptr.
(/) Fixed.
* We shouldn't check the port on defaultFS before reporting error. The defaultFS might point
to an HA name. Just check that defaultFS isn't empty, and report it if it is.
(/) Done.
* For simplicity, cat should just use file->Read rather than PositionRead
(/) Done.
* As an example, gendirs should definitely not need to use fs/namenode_operations
(/) Fixed by adding hardcoded permissions instead of pulling the default ones from the namenode_ops.
* It should not need to use common/* either; we should push the configuration stuff into hdfspp/
(but perhaps that can be a different bug)
(i) Created a new jira for this: HDFS-10787
* We generally don't make values on the stack const (e.g. path, depth, fanout). It's not wrong,
just generally redundant (unless it's important they be const for some reason)
(/) Fixed.
* Put a comment on setting the timeout referring to HDFS-10781 (and vice-versa)
(/) Comments added.
* Add a comment on why we're calling setDefaultSearchPath in the ctor
(/) Added.
* I might make the comment something along the lines of "Creates a configuration loader with
the default search path (....). If you want to explicitly set the entire search path, call
ClearSearchPath() first"
(/) Fixed.
* Filesystem.cc: can SetOwner/SetPermission be written as a call to ::Find(recursive=true)
with the SetOwner/SetPerimission implemented in the callback? Then we wouldn't need three
separate implementations of the recursion logic
(/) Done. Recursive SetOwner and SetPermission now just use Find results to launch their own
asynchrous calls. The code between them is similar though, and might need to be collapsed
even further in the future.
* Does recursive SetOwner/SetPermissions accept globs both for the recursive and non-recursive
versions? We should be consistent. Perhaps SetOwner(explicit filename, fast) and SetOwner(glob/recursive,
slower) should be different methods
(i) Since SetOwner and SetPermission are now using Find to recurse the results, we get wild-cards
for free. To activate them we will possibly need to split all recursive functions in two.
Tools impl:
* Make a usage() function that prints out the usage of the tool. Call it if "--help", "-h",
or a parameter problem occurs.h
(/) Added a usage() function which is called when parameter problem occurs, or when "-h" flag
is passed.
(i) GetOpt does not allow long options, so I did not add "--help" flag, just "-h".
* Keep gendirs in the examples. I don't think we need a tool version of it.
(/) hdfs_gendirs removed.
* Include a comment on HDFS-9539 to fix up the tools and examples as part of the scope.
(/) Done.
* hdfsNewBuilderFromDirectory (in hdfs.cc) should call ClearSearchPath rather than inheriting
the default. Are there any other instances in our codebase where we're currently constructing
loaders whose behavior we need to double-check?
(/) Done. Also fixed in files: configuration_test.cc, configuration_test.h, and hdfs_configuration_test.cc.

> libhdfs++: Create tools directory and implement hdfs_cat, hdfs_chgrp, hdfs_chown, hdfs_chmod
and hdfs_find.
> -----------------------------------------------------------------------------------------------------------
>                 Key: HDFS-10754
>                 URL: https://issues.apache.org/jira/browse/HDFS-10754
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: hdfs-client
>            Reporter: Anatoli Shein
>            Assignee: Anatoli Shein
>         Attachments: HDFS-10754.HDFS-8707.000.patch, HDFS-10754.HDFS-8707.001.patch,
HDFS-10754.HDFS-8707.002.patch, HDFS-10754.HDFS-8707.003.patch, HDFS-10754.HDFS-8707.004.patch,
HDFS-10754.HDFS-8707.005.patch, HDFS-10754.HDFS-8707.006.patch, HDFS-10754.HDFS-8707.007.patch,

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: hdfs-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-help@hadoop.apache.org

View raw message