(tez) branch master updated: TEZ-4413: Fix permission may not be set after crash. (#368). (Ayush Saxena, reviewed by Laszlo Bodor)

[ayushsaxena]

This is an automated email from the ASF dual-hosted git repository.

ayushsaxena pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tez.git


The following commit(s) were added to refs/heads/master by this push:
     new 47997e92b TEZ-4413: Fix permission may not be set after crash. (#368). 
(Ayush Saxena, reviewed by Laszlo Bodor)
47997e92b is described below

commit 47997e92b6cd525f35552380f527b9fa3e8ae466
Author: Ayush Saxena <ayushsax...@apache.org>
AuthorDate: Fri Sep 6 18:07:57 2024 +0530

    TEZ-4413: Fix permission may not be set after crash. (#368). (Ayush Saxena, 
reviewed by Laszlo Bodor)
---
 .../history/logging/proto/DatePartitionedLogger.java | 13 ++++++++++++-
 .../proto/TestProtoHistoryLoggingService.java        | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git 
a/tez-plugins/tez-protobuf-history-plugin/src/main/java/org/apache/tez/dag/history/logging/proto/DatePartitionedLogger.java
 
b/tez-plugins/tez-protobuf-history-plugin/src/main/java/org/apache/tez/dag/history/logging/proto/DatePartitionedLogger.java
index a569567d1..ee838646f 100644
--- 
a/tez-plugins/tez-protobuf-history-plugin/src/main/java/org/apache/tez/dag/history/logging/proto/DatePartitionedLogger.java
+++ 
b/tez-plugins/tez-protobuf-history-plugin/src/main/java/org/apache/tez/dag/history/logging/proto/DatePartitionedLogger.java
@@ -18,6 +18,7 @@
 
 package org.apache.tez.dag.history.logging.proto;
 
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
@@ -71,10 +72,20 @@ public class DatePartitionedLogger<T extends MessageLite> {
 
   private void createDirIfNotExists(Path path) throws IOException {
     FileSystem fileSystem = path.getFileSystem(conf);
+    FileStatus fileStatus = null;
     try {
-      if (!fileSystem.exists(path)) {
+      fileStatus = fileSystem.getFileStatus(path);
+    } catch (FileNotFoundException fnf) {
+      // ignore: regardless of the outcome of FileSystem.getFileStatus call 
(exception or returning null),
+      // we handle the fileStatus == null case later on, so it's safe to 
ignore this exception now
+    }
+    try {
+      if (fileStatus == null) {
         fileSystem.mkdirs(path);
         fileSystem.setPermission(path, DIR_PERMISSION);
+      } else if (!fileStatus.getPermission().equals(DIR_PERMISSION)) {
+        LOG.info("Permission on path {} is {}, setting it to {}", path, 
fileStatus.getPermission(), DIR_PERMISSION);
+        fileSystem.setPermission(path, DIR_PERMISSION);
       }
     } catch (IOException e) {
       // Ignore this exception, if there is a problem it'll fail when trying 
to read or write.
diff --git 
a/tez-plugins/tez-protobuf-history-plugin/src/test/java/org/apache/tez/dag/history/logging/proto/TestProtoHistoryLoggingService.java
 
b/tez-plugins/tez-protobuf-history-plugin/src/test/java/org/apache/tez/dag/history/logging/proto/TestProtoHistoryLoggingService.java
index 4f24d30a8..f599e61d2 100644
--- 
a/tez-plugins/tez-protobuf-history-plugin/src/test/java/org/apache/tez/dag/history/logging/proto/TestProtoHistoryLoggingService.java
+++ 
b/tez-plugins/tez-protobuf-history-plugin/src/test/java/org/apache/tez/dag/history/logging/proto/TestProtoHistoryLoggingService.java
@@ -32,7 +32,10 @@ import java.util.List;
 
 import com.google.protobuf.CodedInputStream;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.util.Time;
 import org.apache.hadoop.yarn.api.records.ApplicationAttemptId;
 import org.apache.hadoop.yarn.api.records.ApplicationId;
 import org.apache.hadoop.yarn.api.records.ContainerId;
@@ -267,6 +270,23 @@ public class TestProtoHistoryLoggingService {
     scanner.close();
   }
 
+  @Test
+  public void testDirPermissions() throws IOException {
+    Path basePath = new Path(tempFolder.newFolder().getAbsolutePath());
+    Configuration conf = new Configuration();
+    FileSystem fs = basePath.getFileSystem(conf);
+    FsPermission expectedPermissions = FsPermission.createImmutable((short) 
01777);
+
+    // Check the directory already exists and doesn't have the expected 
permissions.
+    Assert.assertTrue(fs.exists(basePath));
+    Assert.assertNotEquals(expectedPermissions, 
fs.getFileStatus(basePath).getPermission());
+
+    new DatePartitionedLogger<>(HistoryEventProto.PARSER, basePath, conf, new 
FixedClock(Time.now()));
+
+    // Check the permissions they should be same as the expected permissions
+    Assert.assertEquals(expectedPermissions, 
fs.getFileStatus(basePath).getPermission());
+  }
+
   private List<DAGHistoryEvent> makeHistoryEvents(TezDAGID dagId,
       ProtoHistoryLoggingService service) {
     List<DAGHistoryEvent> historyEvents = new ArrayList<>();