[jira] [Work logged] (GOBBLIN-1785) retention on mr jars dir

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/GOBBLIN-1785?focusedWorklogId=857469&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-857469 ]

ASF GitHub Bot logged work on GOBBLIN-1785:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 17/Apr/23 20:49
            Start Date: 17/Apr/23 20:49
    Worklog Time Spent: 10m 
      Work Description: Will-Lo commented on code in PR #3642:
URL: https://github.com/apache/gobblin/pull/3642#discussion_r1169250482


##########
gobblin-api/src/main/java/org/apache/gobblin/configuration/ConfigurationKeys.java:
##########
@@ -678,7 +678,9 @@ public class ConfigurationKeys {
    */
   public static final String MR_JOB_ROOT_DIR_KEY = "mr.job.root.dir";
   /** Specifies a static location in HDFS to upload jars to. Useful for sharing jars across different Gobblin runs.*/
+  @Deprecated // Deprecated; use MR_JARS_BASE_DIR instead
   public static final String MR_JARS_DIR = "mr.jars.dir";
+  public static final String MR_JARS_BASE_DIR = "mr.jars.base.dir";

Review Comment:
   Can you include here what is the benefit of using this new configuration key over the old configuration?



##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/mapreduce/MRJobLauncher.java:
##########
@@ -228,8 +231,17 @@ public MRJobLauncher(Properties jobProps, Configuration conf, SharedResourcesBro
       this.fs.delete(this.mrJobDir, true);
     }
     this.unsharedJarsDir = new Path(this.mrJobDir, JARS_DIR_NAME);
-    this.jarsDir = this.jobProps.containsKey(ConfigurationKeys.MR_JARS_DIR) ? new Path(
-        this.jobProps.getProperty(ConfigurationKeys.MR_JARS_DIR)) : this.unsharedJarsDir;
+
+    if (this.jobProps.containsKey(ConfigurationKeys.MR_JARS_BASE_DIR)) {
+      Path jarsBaseDir = new Path(this.jobProps.getProperty(ConfigurationKeys.MR_JARS_BASE_DIR));
+      String monthSuffix = new SimpleDateFormat("yyyy-MM").format(System.currentTimeMillis());
+      cleanUpOldJarsDirIfRequired(this.fs, jarsBaseDir);
+      this.jarsDir = new Path(jarsBaseDir, monthSuffix);
+    } else {

Review Comment:
   I think this would be easier having an else-if,
   ```
   } else if (this.jobProps.containsKey(ConfigurationKeys.MR_JARS_DIR)) { 
     this.jarsDir = this.jobProps.getProperty(ConfigurationKeys.MR_JARS_DIR);
   } else {
     this.jarsDir = this.unsharedJarsDir;
   }
   ```





Issue Time Tracking
-------------------

            Worklog Id:     (was: 857469)
    Remaining Estimate: 0h
            Time Spent: 10m

> retention on mr jars dir
> ------------------------
>
>                 Key: GOBBLIN-1785
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-1785
>             Project: Apache Gobblin
>          Issue Type: Improvement
>            Reporter: Arjun Singh Bora
>            Priority: Major
>          Time Spent: 10m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)