hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hemanth Yamijala (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HADOOP-4930) Implement setuid executable for Linux to assist in launching tasks as job owners
Date Fri, 26 Dec 2008 05:24:44 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-4930?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659223#action_12659223
] 

Hemanth Yamijala commented on HADOOP-4930:
------------------------------------------

This is looking good. But I do have some more comments.

- main seems to assume the same number of arguments for all calls to the taskcontroller. While
that is true for the two commands we implemented currently, it might not be true in future.
It may be better to move the arguments processing per command.
- umask needs to be set before launching the executable. This is because the child java process
creates files that should have the right permissions.
- Most of the comments that remain are related to the configuration parsing. I have two suggestions:
-- As a first option, if we can find a package that would do this parsing automatically for
us, and that package is easy to include with Hadoop (or better still, already is or is widely
available), then that would be great to use, and we can just do away with all the complex
parsing logic.
-- If not, at a minimum, let's move all configuration related code to a separate file so that
it can be managed easily.
- Some configuration related comments:
-- It seems good to have all state of configuration in a separate struct, something like:
{code}
struct configuration {
  int size;
  struct confentry** confdetails;
};
{code}
-- get_value is returning nothing when there's an error in get_configs, where a NULL needs
to be returned.
-- the buffer length passed to the fgets call should actually be decremented by the amount
already read, since fgets will return on seeing a newline immediately.
-- strncat does not specifically return any error related values. The check for NULL seems
unnecessary.
-- The config entries are not being freed correctly on error conditions. Basically, free_configurations
should be enhanced to handle partially allocated entries.
-- The contents buffer should not be freed at the end of parsing, but just before the exec
call. This is because configuration is holding pointers to the buffer as keys and values.
-- search_conf variable is unused.
-- It will be more conventional to have the length of strings being calculated to have the
exact count, and add a character for null termination only in the malloc / realloc calls.
This way, other calls like snprintf can use strlen directly without having to worry about
the null character. 
-- It is probably also safer to memset allocated buffers with 0s.

> Implement setuid executable for Linux to assist in launching tasks as job owners
> --------------------------------------------------------------------------------
>
>                 Key: HADOOP-4930
>                 URL: https://issues.apache.org/jira/browse/HADOOP-4930
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: mapred
>            Reporter: Hemanth Yamijala
>            Assignee: Sreekanth Ramakrishnan
>         Attachments: HADOOP-4930-1.patch, HADOOP-4930-2.patch, HADOOP-4930-3.patch, HADOOP-4930-4.patch
>
>
> HADOOP-4490 tracks the design and implementation of launching tasks as job owners. As
per discussion there, the proposal is to implement a setuid executable for Linux that would
be launched by the tasktracker to run tasks as the job owners. In this task we will track
the implementation of the setuid executable. HADOOP-4490 will be used to track changes in
the tasktracker.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message