You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "Luca Telloli (JIRA)" <ji...@apache.org> on 2009/05/14 11:22:45 UTC

[jira] Created: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Process dfs.name.edits.dirs as URI 
-----------------------------------

                 Key: HADOOP-5832
                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
             Project: Hadoop Core
          Issue Type: Sub-task
            Reporter: Luca Telloli


Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12711916#action_12711916 ] 

Konstantin Shvachko commented on HADOOP-5832:
---------------------------------------------

This comment is relevant to this issue.
[http://issues.apache.org/jira/browse/HADOOP-5188#action_12711913]

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>            Reporter: Luca Telloli
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12710270#action_12710270 ] 

dhruba borthakur commented on HADOOP-5832:
------------------------------------------

This looks like a good one to have !

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>            Reporter: Luca Telloli
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Affects Version/s: 0.20.0
        Fix Version/s: 0.21.0

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12720388#action_12720388 ] 

Raghu Angadi commented on HADOOP-5832:
--------------------------------------

Thanks for the changes. Looks better. 

nit: Could you move the block of code that checks for consistency (including the check to see if it file://) a method? It is repeated for dirs and edits (it around 20 lines).


> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Attachment: HADOOP-5832.patch

Raghu, thanks for the comments they're helpful to me for making good progress! 

- I factored out the repeated code 
- I added a consistency check also for properties fs.checkpoint.dir and fs.checkpoint.edits.dir that wasn't currently in place 

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Attachment: HADOOP-5832.patch

Addressed the issue of previous comments; in particular: 
- fixed the failing test 
- fixed the indentation issue; since most of the work have been done manually, there's a couple of places where the code doesn't change, but only the indentation; I apologize in advance for those lines but at the same time I don't think I should revert them, since the bad indentation was there before the patch, and it's resolved afterwards. Additionally, as I said, the number of lines where this happens is very limited. 

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Issue Comment Edited: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12719323#action_12719323 ] 

Luca Telloli edited comment on HADOOP-5832 at 6/15/09 2:06 AM:
---------------------------------------------------------------

> Could you confirm if any of the error handling is different compared current trunk? If there are any differences could you list them here? 

My approach has been to catch any new exception and throw a new IOException with its content 

> Please update hdfs-default.xml for dfs.name.dir. Mainly the description needs to be updated for new syntax.

Done

> May be a warning when there is no schema would help get configs updated.

Done 

> Indentation "fixes" : I don't see any reason do indentation fixes far away from actual changes for this patch. Some of these are not even fixes. A good rule of thumb I follow is that if I have a hunk in the patch that has only these changes, I remove it from my patch. That is easy to do. On the plus side, you won't get 'svn blame'd for bugs around there .

It's a consequence of using Eclipse in a different configuration. Just setting the values of tabstop to 2 and whitespace instead of tab proved not to be sufficient to avoid these situations. Although I tried to fix most of them manually, there might be a couple of leftovers. 

Finally, I think the approach should be: 
- non URI: consider it valid, use as file:// and warn the user, as you suggested
- unknown uri scheme: the scheme should be among the ones specified inside the JournalType enumeration, otherwise hard error 

I'm posting a new patch that fixes the issues you stated above, please let me know your impressions on it

      was (Author: lucat):
    > Could you confirm if any of the error handling is different compared current trunk? If there are any differences could you list them here? 

My approach has been to catch any new exception and throw a new IOException with its content 

> Please update hdfs-default.xml for dfs.name.dir. Mainly the description needs to be updated for new syntax.

Done

> May be a warning when there is no schema would help get configs updated.

Done 

> Indentation "fixes" : I don't see any reason do indentation fixes far away from actual changes for this patch. Some of these are not even fixes. A good rule of thumb I follow is that if I have a hunk in the patch that has only these changes, I remove it from my patch. That is easy to do. On the plus side, you won't get 'svn blame'd for bugs around there .

It's a consequence of using Eclipse in a different configuration. Just setting the values of tabstop to 2 and whitespace instead of tab proved not to be sufficient to avoid these situations. Although I tried to fix most of them manually, there might be a couple of leftovers. 

Finally, I think the approach should be: 
- non URI: consider it valid, use as file:// and warn the user, as you suggested
- unknown uri scheme: the usi should be specified inside the JournalType enumeration, otherwise hard error 

I'm posting a new patch that fixes the issues you stated above, please let me know your impressions on it
  
> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Status: Patch Available  (was: Open)

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12716724#action_12716724 ] 

Raghu Angadi commented on HADOOP-5832:
--------------------------------------

This is a pretty spread out patch. Could you fix the indentation in multiple places? There are patch guidelines on the wiki, but two most important ones are :
# use spaces instead of tabs, and
# use indentation of 2.

It would matter less if the patch was a small isolated change.

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Attachment: HADOOP-5832.patch

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Status: Open  (was: Patch Available)

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Status: Open  (was: Patch Available)

cancelling for later resubmission due to latest comments

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Status: Open  (was: Patch Available)

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Status: Patch Available  (was: Open)

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12718988#action_12718988 ] 

Flavio Paiva Junqueira commented on HADOOP-5832:
------------------------------------------------

Raghu, being able to specify non-file URIs is part of the point of this patch. As Luca is working on it towards the integration of BookKeeper (HADOOP-5189), we would like to eventually be able to specify a URI for non-file types of logging such as BookKeeper.

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12716628#action_12716628 ] 

Benjamin Reed commented on HADOOP-5832:
---------------------------------------

+1 looks good

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12718130#action_12718130 ] 

Flavio Paiva Junqueira commented on HADOOP-5832:
------------------------------------------------

There is an issue open about TestHdfsProxy failing (HADOOP-6007), so it sounds like it is a problem with the test.

If no one else has any other comment on the patch, it would be great to have it committed.

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12717460#action_12717460 ] 

Hadoop QA commented on HADOOP-5832:
-----------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12410084/HADOOP-5832.patch
  against trunk revision 782708.

    +1 @author.  The patch does not contain any @author tags.

    +1 tests included.  The patch appears to include 23 new or modified tests.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac compiler warnings.

    +1 findbugs.  The patch does not introduce any new Findbugs warnings.

    +1 Eclipse classpath. The patch retains Eclipse classpath integrity.

    +1 release audit.  The applied patch does not increase the total number of release audit warnings.

    -1 core tests.  The patch failed core unit tests.

    -1 contrib tests.  The patch failed contrib unit tests.

Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/481/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/481/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/481/artifact/trunk/build/test/checkstyle-errors.html
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-vesta.apache.org/481/console

This message is automatically generated.

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Attachment: HADOOP-5832.patch

Attaching a specific patch for this issue, since HADOOP-5188 has not been fully agreed so far. This should also help the review process

This patch allows passing arguments of the properties dfs.name.dir and dfs.name.edits.dir as URI, along with fs.checkpoint.dir and fs.checkpoint.edits.dir. 
The patch supports current (plain) directories as values, assuming that they refer to a file:// type of URI. 



> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12717668#action_12717668 ] 

Luca Telloli commented on HADOOP-5832:
--------------------------------------

I checked the two tests which failed in hudson:

- mapred/TestReduceFetch failed on my node with trunk *both* patched  and unpatched, so the problem seems related to some previous issue. Btw, on Hudson, this test was timing out, on my box it's failing due to an error. 

- contrib/TestHdfsProxy passes without issues on my node; as far a I can see this test has been failing on other patches previous to this one so again I'm not sure if its outcome is related to this patch at all 




> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Status: Patch Available  (was: Open)

resubmitting since it looks there's issues with hudson 

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12718979#action_12718979 ] 

Raghu Angadi commented on HADOOP-5832:
--------------------------------------

What happens if someone specifies  a non-file URI with this patch? I would say any unsported URI should result in a hard error.
 

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12717097#action_12717097 ] 

Flavio Paiva Junqueira commented on HADOOP-5832:
------------------------------------------------

+1, it looks good to me. I've only been able to find 3 instances in which this patch replaces an existing tab with the correct indentation. HDFS tests pass fine for me. Thanks, Luca!



> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Issue Comment Edited: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12716625#action_12716625 ] 

Luca Telloli edited comment on HADOOP-5832 at 6/5/09 7:38 AM:
--------------------------------------------------------------

Attaching a specific patch for this issue, since HADOOP-5188 has not been fully agreed so far. This should also help the review process

This patch allows passing arguments of the properties dfs.name.dir and dfs.name.edits.dir as URI, along with fs.checkpoint.dir and fs.checkpoint.edits.dir. 
The patch supports current (plain) directories as values, assuming that they refer to a file:// type of URI. 

This patch should also help HADOOP-5188, for the deployment of different types of logging than files. 

      was (Author: lucat):
    Attaching a specific patch for this issue, since HADOOP-5188 has not been fully agreed so far. This should also help the review process

This patch allows passing arguments of the properties dfs.name.dir and dfs.name.edits.dir as URI, along with fs.checkpoint.dir and fs.checkpoint.edits.dir. 
The patch supports current (plain) directories as values, assuming that they refer to a file:// type of URI. 


  
> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12717467#action_12717467 ] 

Raghu Angadi commented on HADOOP-5832:
--------------------------------------

> It is true that the patch is spread out, but most of it replaces the use of File with URI, and touching multiple files seems necessary to add the proposed functionality. I don't see a good way around it, so it is fine with me.
 
Right. I meant since it is a spread out patch, indentation is more important.

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Erik Eldridge (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12718134#action_12718134 ] 

Erik Eldridge commented on HADOOP-5832:
---------------------------------------

Hi,

I will be out of the office at the Hadoop Summit :) on 6/10.  I'll be  
checking email occassionaly.  For urgent matters, please try my cell  
phone (it's on backyard).

Thanks!

On Jun 10, 2009, at 9:30 AM, "Flavio Paiva Junqueira (JIRA)" <jira@apache.org 



> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Luca Telloli updated HADOOP-5832:
---------------------------------

    Hadoop Flags:   (was: [Incompatible change])
          Status: Patch Available  (was: Open)

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>            Reporter: Luca Telloli
>         Attachments: HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Luca Telloli (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12719323#action_12719323 ] 

Luca Telloli commented on HADOOP-5832:
--------------------------------------

> Could you confirm if any of the error handling is different compared current trunk? If there are any differences could you list them here? 

My approach has been to catch any new exception and throw a new IOException with its content 

> Please update hdfs-default.xml for dfs.name.dir. Mainly the description needs to be updated for new syntax.

Done

> May be a warning when there is no schema would help get configs updated.

Done 

> Indentation "fixes" : I don't see any reason do indentation fixes far away from actual changes for this patch. Some of these are not even fixes. A good rule of thumb I follow is that if I have a hunk in the patch that has only these changes, I remove it from my patch. That is easy to do. On the plus side, you won't get 'svn blame'd for bugs around there .

It's a consequence of using Eclipse in a different configuration. Just setting the values of tabstop to 2 and whitespace instead of tab proved not to be sufficient to avoid these situations. Although I tried to fix most of them manually, there might be a couple of leftovers. 

Finally, I think the approach should be: 
- non URI: consider it valid, use as file:// and warn the user, as you suggested
- unknown uri scheme: the usi should be specified inside the JournalType enumeration, otherwise hard error 

I'm posting a new patch that fixes the issues you stated above, please let me know your impressions on it

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12716882#action_12716882 ] 

Flavio Paiva Junqueira commented on HADOOP-5832:
------------------------------------------------

On the comments from Raghu:

* -1 on indentation. I'm seeing tabs in some included lines, and some statements seem misplaced, like the "finally" declaration in TestBackupNode.java;
* It is true that the patch is spread out, but most of it replaces the use of File with URI, and touching multiple files seems necessary to add the proposed functionality. I don't see a good way around it, so it is fine with me.

I have run the hdfs tests, and it passes all but one: Test org.apache.hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer

It gives me the following error:

{noformat}
Testcase: testOIV took 7.667 sec
	Caused an ERROR
null
java.lang.ArrayStoreException
	at java.lang.System.arraycopy(Native Method)
	at java.util.Arrays.copyOf(Arrays.java:2763)
	at java.util.ArrayList.toArray(ArrayList.java:305)
	at org.apache.hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer.initFsimage(TestOfflineImageViewer.java:127)
	at org.apache.hadoop.hdfs.tools.offlineImageViewer.TestOfflineImageViewer.testOIV(TestOfflineImageViewer.java:77)
{noformat}

Inspecting the code, we see that line 127 of the test is:

{code:title=TestOfflineImageViewer.java|borderStyle=solid}
File [] files = cluster.getNameDirs().toArray(new File[0]);
{code} 

which looks like it could be caused by the changes of this patch. In fact, without the patch, this test runs fine for me.

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Commented: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12718999#action_12718999 ] 

Raghu Angadi commented on HADOOP-5832:
--------------------------------------

Luca,

Could you confirm if any of the error handling is different compared current trunk? If there are any differences could you list them here? It is harder reviewers alone to do this. Could you confirm that things that caused a hard error still does and vice versa.
For e.g:  {noformat}
      try {
        URI u = new URI(name);
        // If the scheme was not declared, default to file://
        // and use the absolute path of the file 
        if(u.getScheme() == null)
          u = new URI("file://" + new File(name).getAbsolutePath());
        dirs.add(u);
      } catch (Exception e) {
        LOG.error("Error while processing URI: " + name + 
            ". The error message was: " + e.getMessage());
      }
{noformat}
Edits and FSImage directories are very critical part of NN operation. Ignoring error like this might not be good.

Other comments : 
# Please update hdfs-default.xml for {{dfs.name.dir}}. Mainly the description needs to be updated for new syntax.
# May be a warning when there is no schema would help get configs updated.
# Indentation "fixes" : I don't see any reason do indentation fixes far away from actual changes for this patch. Some of these are not even fixes. A good rule of thumb I follow is that if I have a hunk in the patch that has only these changes, I remove it from my patch. That is easy to do. On the plus side, you won't get 'svn blame'd for bugs around there :).





> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Konstantin Shvachko updated HADOOP-5832:
----------------------------------------

    Comment: was deleted

(was: Hi,

I will be out of the office at the Hadoop Summit :) on 6/10.  I'll be  
checking email occassionaly.  For urgent matters, please try my cell  
phone (it's on backyard).

Thanks!

On Jun 10, 2009, at 9:30 AM, "Flavio Paiva Junqueira (JIRA)" <jira@apache.org 

)

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>    Affects Versions: 0.20.0
>            Reporter: Luca Telloli
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5832.patch, HADOOP-5832.patch
>
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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


[jira] Updated: (HADOOP-5832) Process dfs.name.edits.dirs as URI

Posted by "dhruba borthakur (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HADOOP-5832?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

dhruba borthakur updated HADOOP-5832:
-------------------------------------

    Component/s: dfs

> Process dfs.name.edits.dirs as URI 
> -----------------------------------
>
>                 Key: HADOOP-5832
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5832
>             Project: Hadoop Core
>          Issue Type: Sub-task
>          Components: dfs
>            Reporter: Luca Telloli
>
> Process the value of property dfs.name.edits.dirs as URI, to allow different schemes than just file. As an advantage, Java supports the constructor File(URI) so the transition is straightforward for files. 

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