You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vaibhav Gumashta <vg...@hortonworks.com> on 2014/09/09 01:23:31 UTC
Review Request 25460: Recursive root scratch directory creation is
not using hdfs umask properly
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25460/
-----------------------------------------------------------
Review request for hive and Thejas Nair.
Bugs: HIVE-8022
https://issues.apache.org/jira/browse/HIVE-8022
Repository: hive-git
Description
-------
Recursive root scratch directory creation is not using hdfs umask properly
Diffs
-----
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 5dce16f
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 5bbf3f6
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 47fe508
Diff: https://reviews.apache.org/r/25460/diff/
Testing
-------
Thanks,
Vaibhav Gumashta
Re: Review Request 25460: Recursive root scratch directory creation
is not using hdfs umask properly
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25460/#review52656
-----------------------------------------------------------
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
<https://reviews.apache.org/r/25460/#comment91541>
can you also add a test for dir with multiple levels of dirs (that need to get created)?
Also, please use System.getProperty("test.tmp.dir"), instead of "/tmp"
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/25460/#comment91537>
I guess this issue might have existed in the old code as well -
We are loosing the error message here, if the mkdirs fails. Maybe change this to
try {
retval = fs.mkdirs(mkdirPath, fsPermission);
resetConf(conf, recursive, origUmask);
} catch (IOException ioe) {
resetConf(conf, recursive, origUmask);
throw ioe;
} finally {
IOUtils.closeStream(fs);
}
A cleaner solution would have been to use Closer in guava, but it requires upgrade to guava 14, I am not sure if you want to go into that as part of this patch- https://code.google.com/p/guava-libraries/wiki/ClosingResourcesExplained
- Thejas Nair
On Sept. 8, 2014, 11:23 p.m., Vaibhav Gumashta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25460/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2014, 11:23 p.m.)
>
>
> Review request for hive and Thejas Nair.
>
>
> Bugs: HIVE-8022
> https://issues.apache.org/jira/browse/HIVE-8022
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Recursive root scratch directory creation is not using hdfs umask properly
>
>
> Diffs
> -----
>
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 5dce16f
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 5bbf3f6
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 47fe508
>
> Diff: https://reviews.apache.org/r/25460/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vaibhav Gumashta
>
>
Re: Review Request 25460: Recursive root scratch directory creation
is not using hdfs umask properly
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
> On Sept. 9, 2014, 10:55 p.m., Thejas Nair wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java, line 3454
> > <https://reviews.apache.org/r/25460/diff/1-2/?file=682938#file682938line3454>
> >
> > this should not be throwing any exception. I assume the code here does not, so the 'throws IOException' can be removed.
> > If it were to throw an exception then that exception would mask the exception from mkdirs.
Ah sorry, missed that.
- Vaibhav
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25460/#review52795
-----------------------------------------------------------
On Sept. 9, 2014, 11:06 p.m., Vaibhav Gumashta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25460/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 11:06 p.m.)
>
>
> Review request for hive and Thejas Nair.
>
>
> Bugs: HIVE-8022
> https://issues.apache.org/jira/browse/HIVE-8022
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Recursive root scratch directory creation is not using hdfs umask properly
>
>
> Diffs
> -----
>
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 5dce16f
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 5bbf3f6
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 08f149a
>
> Diff: https://reviews.apache.org/r/25460/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vaibhav Gumashta
>
>
Re: Review Request 25460: Recursive root scratch directory creation
is not using hdfs umask properly
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25460/#review52795
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
<https://reviews.apache.org/r/25460/#comment91926>
this should not be throwing any exception. I assume the code here does not, so the 'throws IOException' can be removed.
If it were to throw an exception then that exception would mask the exception from mkdirs.
- Thejas Nair
On Sept. 9, 2014, 9:41 p.m., Vaibhav Gumashta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25460/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 9:41 p.m.)
>
>
> Review request for hive and Thejas Nair.
>
>
> Bugs: HIVE-8022
> https://issues.apache.org/jira/browse/HIVE-8022
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Recursive root scratch directory creation is not using hdfs umask properly
>
>
> Diffs
> -----
>
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 5dce16f
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 5bbf3f6
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 08f149a
>
> Diff: https://reviews.apache.org/r/25460/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vaibhav Gumashta
>
>
Re: Review Request 25460: Recursive root scratch directory creation
is not using hdfs umask properly
Posted by Thejas Nair <th...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25460/#review52807
-----------------------------------------------------------
Ship it!
Ship It!
- Thejas Nair
On Sept. 9, 2014, 11:06 p.m., Vaibhav Gumashta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25460/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 11:06 p.m.)
>
>
> Review request for hive and Thejas Nair.
>
>
> Bugs: HIVE-8022
> https://issues.apache.org/jira/browse/HIVE-8022
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Recursive root scratch directory creation is not using hdfs umask properly
>
>
> Diffs
> -----
>
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 5dce16f
> ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 5bbf3f6
> ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 08f149a
>
> Diff: https://reviews.apache.org/r/25460/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vaibhav Gumashta
>
>
Re: Review Request 25460: Recursive root scratch directory creation
is not using hdfs umask properly
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25460/
-----------------------------------------------------------
(Updated Sept. 9, 2014, 11:06 p.m.)
Review request for hive and Thejas Nair.
Bugs: HIVE-8022
https://issues.apache.org/jira/browse/HIVE-8022
Repository: hive-git
Description
-------
Recursive root scratch directory creation is not using hdfs umask properly
Diffs (updated)
-----
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 5dce16f
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 5bbf3f6
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 08f149a
Diff: https://reviews.apache.org/r/25460/diff/
Testing
-------
Thanks,
Vaibhav Gumashta
Re: Review Request 25460: Recursive root scratch directory creation
is not using hdfs umask properly
Posted by Vaibhav Gumashta <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25460/
-----------------------------------------------------------
(Updated Sept. 9, 2014, 9:41 p.m.)
Review request for hive and Thejas Nair.
Bugs: HIVE-8022
https://issues.apache.org/jira/browse/HIVE-8022
Repository: hive-git
Description
-------
Recursive root scratch directory creation is not using hdfs umask properly
Diffs (updated)
-----
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 5dce16f
ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 5bbf3f6
ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 08f149a
Diff: https://reviews.apache.org/r/25460/diff/
Testing
-------
Thanks,
Vaibhav Gumashta