You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tez.apache.org by GitBox <gi...@apache.org> on 2022/05/03 11:57:47 UTC
[GitHub] [tez] skysiders opened a new pull request, #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
skysiders opened a new pull request, #209:
URL: https://github.com/apache/tez/pull/209
TEZ-4412 ensure mkDirForAM create directory with special permissions
I found TezClientUtils has method ensureStagingDirExists.It will check whether the path "stagingArea" exists, if it exists, check the permission, if not, call the function "TezCommonUtils.mkDirForAM" to create.But in method mkDirForAM,it use mkdir(path, permission) to create, if umask too strict such as 777,this directory will set with 000 permission.So we need to ensure the directory has right permission.
In this patch, I compared the permissions of the files with the expected permissions. If they are inconsistent, then log this and use setPermission to grant directory permissions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] abstractdog commented on a diff in pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
abstractdog commented on code in PR #209:
URL: https://github.com/apache/tez/pull/209#discussion_r868831466
##########
tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java:
##########
@@ -413,4 +414,16 @@ public void testGetDAGSessionTimeout() {
}
+ @Test
+ public void testMkDirForAM() throws IOException {
+ Configuration remoteConf = new Configuration();
+ remoteConf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR);
+ remoteConf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "777");
+ MiniDFSCluster miniDFS = new MiniDFSCluster.Builder(remoteConf).numDataNodes(3).format(true).racks(null)
Review Comment:
merged to master, thanks @skysiders for this patch, your tireless work, and quick responses!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] skysiders commented on a diff in pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
skysiders commented on code in PR #209:
URL: https://github.com/apache/tez/pull/209#discussion_r867684241
##########
tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java:
##########
@@ -413,4 +414,16 @@ public void testGetDAGSessionTimeout() {
}
+ @Test
+ public void testMkDirForAM() throws IOException {
+ Configuration remoteConf = new Configuration();
+ remoteConf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR);
+ remoteConf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "777");
+ MiniDFSCluster miniDFS = new MiniDFSCluster.Builder(remoteConf).numDataNodes(3).format(true).racks(null)
Review Comment:
Hi @abstractdog .Thanks for your suggestion, In this patch, I closed miniDFS after I ran out of it. Thank you for reviewing my code, it has taught me a lot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] abstractdog commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
abstractdog commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1120199998
thanks @skysiders , there is 1 more checkstyle issue reported
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] skysiders commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
skysiders commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1120202580
Thanks for your review @abstractdog , I use checkstyle:check find issue in `import`. I used the new FsPermission(700) check as assert at first, but then I found it would be more appropriate to use TezCommonUtils.TEZ_AM_DIR_PERMISSION, so I replaced the content in the assert, and forgot to delete the import.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] tez-yetus commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
tez-yetus commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1116055863
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 16m 55s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 15m 57s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 42s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| +1 :green_heart: | javadoc | 0m 55s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 44s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +0 :ok: | spotbugs | 1m 41s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 22s | the patch passed |
| +1 :green_heart: | compile | 0m 25s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 25s | the patch passed |
| +1 :green_heart: | compile | 0m 22s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 22s | the patch passed |
| -0 :warning: | checkstyle | 0m 13s | tez-api: The patch generated 5 new + 15 unchanged - 0 fixed = 20 total (was 15) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 24s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | findbugs | 1m 1s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 1s | tez-api in the patch passed. |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 45m 7s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/209 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 0eb59b1508ee 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 75876fcc7 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/1/artifact/out/diff-checkstyle-tez-api.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/1/testReport/ |
| Max. process+thread count | 296 (vs. ulimit of 5500) |
| modules | C: tez-api U: tez-api |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/1/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] skysiders commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
skysiders commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1118272618
Hi @abstractdog ,I found some permission issue here, can you please take a look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] abstractdog commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
abstractdog commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1120160301
thanks for the patch @skysiders!
looks good to me, left a minor comment, please take care of it as well as checkstyle warnings
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] tez-yetus commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
tez-yetus commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1120177010
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 2s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 17m 5s | master passed |
| +1 :green_heart: | compile | 0m 44s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 41s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 10s | master passed |
| +1 :green_heart: | javadoc | 0m 56s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 44s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +0 :ok: | spotbugs | 1m 39s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 37s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 23s | the patch passed |
| +1 :green_heart: | compile | 0m 26s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 26s | the patch passed |
| +1 :green_heart: | compile | 0m 21s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 21s | the patch passed |
| -0 :warning: | checkstyle | 0m 13s | tez-api: The patch generated 1 new + 23 unchanged - 0 fixed = 24 total (was 23) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | findbugs | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 5s | tez-api in the patch passed. |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 30m 28s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/209 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux e757c91223e4 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 798ddda06 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/3/artifact/out/diff-checkstyle-tez-api.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/3/testReport/ |
| Max. process+thread count | 398 (vs. ulimit of 5500) |
| modules | C: tez-api U: tez-api |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/3/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] skysiders commented on a diff in pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
skysiders commented on code in PR #209:
URL: https://github.com/apache/tez/pull/209#discussion_r867321331
##########
tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java:
##########
@@ -291,7 +291,14 @@ public static Path getSummaryRecoveryPath(Path attemptRecoverPath) {
* @throws IOException
*/
public static void mkDirForAM(FileSystem fs, Path dir) throws IOException {
- fs.mkdirs(dir, new FsPermission(TEZ_AM_DIR_PERMISSION));
+ FsPermission perm = new FsPermission(TEZ_AM_DIR_PERMISSION);
+ fs.mkdirs(dir, perm);
+ if(!fs.getFileStatus(dir).getPermission().equals(perm)){
Review Comment:
Thanks for your review. I will check it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] tez-yetus commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
tez-yetus commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1120205702
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 59s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 16m 11s | master passed |
| +1 :green_heart: | compile | 0m 45s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 41s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | javadoc | 0m 56s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 44s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +0 :ok: | spotbugs | 1m 38s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 36s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 23s | the patch passed |
| +1 :green_heart: | compile | 0m 26s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 26s | the patch passed |
| +1 :green_heart: | compile | 0m 22s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 22s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 13s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | findbugs | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 6s | tez-api in the patch passed. |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 29m 32s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/209 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux e045ab3e49ea 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 798ddda06 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/4/testReport/ |
| Max. process+thread count | 391 (vs. ulimit of 5500) |
| modules | C: tez-api U: tez-api |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/4/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] abstractdog merged pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
abstractdog merged PR #209:
URL: https://github.com/apache/tez/pull/209
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] abstractdog commented on a diff in pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
abstractdog commented on code in PR #209:
URL: https://github.com/apache/tez/pull/209#discussion_r867666466
##########
tez-api/src/test/java/org/apache/tez/common/TestTezCommonUtils.java:
##########
@@ -413,4 +414,16 @@ public void testGetDAGSessionTimeout() {
}
+ @Test
+ public void testMkDirForAM() throws IOException {
+ Configuration remoteConf = new Configuration();
+ remoteConf.set(MiniDFSCluster.HDFS_MINIDFS_BASEDIR, TEST_ROOT_DIR);
+ remoteConf.set(CommonConfigurationKeys.FS_PERMISSIONS_UMASK_KEY, "777");
+ MiniDFSCluster miniDFS = new MiniDFSCluster.Builder(remoteConf).numDataNodes(3).format(true).racks(null)
Review Comment:
unit test works as expected, this patch is so close, 1 more thing, could you please shutdown miniDFS cluster?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] tez-yetus commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
tez-yetus commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1120733973
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 2s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 15m 58s | master passed |
| +1 :green_heart: | compile | 0m 46s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 42s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| +1 :green_heart: | javadoc | 0m 56s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 44s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +0 :ok: | spotbugs | 1m 39s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 36s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 23s | the patch passed |
| +1 :green_heart: | compile | 0m 27s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 27s | the patch passed |
| +1 :green_heart: | compile | 0m 23s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 23s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 14s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | findbugs | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 6s | tez-api in the patch passed. |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 29m 24s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/209 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux 0801a38611fa 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 798ddda06 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/5/testReport/ |
| Max. process+thread count | 423 (vs. ulimit of 5500) |
| modules | C: tez-api U: tez-api |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/5/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] tez-yetus commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
tez-yetus commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1120169848
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 2s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | The patch appears to include 1 new or modified test files. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 16m 28s | master passed |
| +1 :green_heart: | compile | 0m 45s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | compile | 0m 42s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | checkstyle | 1m 10s | master passed |
| +1 :green_heart: | javadoc | 0m 55s | master passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 44s | master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +0 :ok: | spotbugs | 1m 38s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 36s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 23s | the patch passed |
| +1 :green_heart: | compile | 0m 26s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javac | 0m 26s | the patch passed |
| +1 :green_heart: | compile | 0m 22s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | javac | 0m 22s | the patch passed |
| -0 :warning: | checkstyle | 0m 13s | tez-api: The patch generated 1 new + 23 unchanged - 0 fixed = 24 total (was 23) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 |
| +1 :green_heart: | javadoc | 0m 23s | the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| +1 :green_heart: | findbugs | 1m 2s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 3s | tez-api in the patch passed. |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 29m 45s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/tez/pull/209 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile |
| uname | Linux be77216af50b 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/tez.sh |
| git revision | master / 798ddda06 |
| Default Java | Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07 |
| checkstyle | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/2/artifact/out/diff-checkstyle-tez-api.txt |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/2/testReport/ |
| Max. process+thread count | 292 (vs. ulimit of 5500) |
| modules | C: tez-api U: tez-api |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-209/2/console |
| versions | git=2.25.1 maven=3.6.3 findbugs=3.0.1 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] skysiders commented on pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
skysiders commented on PR #209:
URL: https://github.com/apache/tez/pull/209#issuecomment-1120172558
> this test is properly working for the original scenario, can you include another one that tests the scenario you mentioned (umask)?
Hi @abstractdog , I'm sorry to make mistake in UT, and I try to fix it in this patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tez] abstractdog commented on a diff in pull request #209: TEZ-4412 ensure mkDirForAM create directory with special permissions
Posted by GitBox <gi...@apache.org>.
abstractdog commented on code in PR #209:
URL: https://github.com/apache/tez/pull/209#discussion_r867321061
##########
tez-api/src/main/java/org/apache/tez/common/TezCommonUtils.java:
##########
@@ -291,7 +291,14 @@ public static Path getSummaryRecoveryPath(Path attemptRecoverPath) {
* @throws IOException
*/
public static void mkDirForAM(FileSystem fs, Path dir) throws IOException {
- fs.mkdirs(dir, new FsPermission(TEZ_AM_DIR_PERMISSION));
+ FsPermission perm = new FsPermission(TEZ_AM_DIR_PERMISSION);
+ fs.mkdirs(dir, perm);
+ if(!fs.getFileStatus(dir).getPermission().equals(perm)){
Review Comment:
this method is public static, looks easily testable, could you please introduce a unit test in TestTezCommonUtils?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org