Fix thread safety issues with SDF (#9425)

apache · Sep 19, 2022 · 353774f · 353774f
1 parent d1ccee6
commit 353774f
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 21 deletions.
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/utils/DateTimeUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/utils/DateTimeUtils.java
@@ -18,8 +18,8 @@
  */
 package org.apache.pinot.common.utils;
 
-import java.text.SimpleDateFormat;
-import java.util.Date;
+import java.time.Instant;
+import java.time.format.DateTimeFormatter;
 import java.util.Locale;
 import java.util.TimeZone;
 
@@ -29,11 +29,8 @@ private DateTimeUtils() {
   }
 
   private static final String DATE_FORMAT = "yyyy-MM-dd HH:mm:ss z";
-  private static final SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat(DATE_FORMAT, Locale.getDefault());
-
-  static {
-    SIMPLE_DATE_FORMAT.setTimeZone(TimeZone.getDefault());
-  }
+  private static final DateTimeFormatter SIMPLE_DATE_FORMAT =
+      DateTimeFormatter.ofPattern(DATE_FORMAT, Locale.getDefault()).withZone(TimeZone.getDefault().toZoneId());
 
   /**
    * Utility function to convert epoch in millis to SDF of form "yyyy-MM-dd HH:mm:ss z".
@@ -42,6 +39,6 @@ private DateTimeUtils() {
    * @return SDF equivalent
    */
   public static String epochToDefaultDateFormat(long millisSinceEpoch) {
-    return SIMPLE_DATE_FORMAT.format(new Date(millisSinceEpoch));
+    return SIMPLE_DATE_FORMAT.format(Instant.ofEpochMilli(millisSinceEpoch));
   }
 }
diff --git a/...oller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java b/...oller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
@@ -28,7 +28,9 @@
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import java.text.SimpleDateFormat;
+import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -41,7 +43,6 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
-import java.util.TimeZone;
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.UUID;
@@ -182,7 +183,8 @@ public class PinotHelixResourceManager {
   public static final long SEGMENT_CLEANUP_TIMEOUT_MS = 20 * 60_000L; // 20 minutes
   public static final long SEGMENT_CLEANUP_CHECK_INTERVAL_MS = 1_000L; // 1 second
 
-  private static final SimpleDateFormat SIMPLE_DATE_FORMAT = new SimpleDateFormat("yyyyMMdd'T'HHmmss'Z'");
+  private static final DateTimeFormatter SIMPLE_DATE_FORMAT =
+      DateTimeFormatter.ofPattern("yyyyMMdd'T'HHmmss'Z'").withZone(ZoneOffset.UTC);
 
   private final Map<String, Map<String, Long>> _segmentCrcMap = new HashMap<>();
   private final Map<String, Map<String, Integer>> _lastKnownSegmentMetadataVersionMap = new HashMap<>();
@@ -231,7 +233,6 @@ public String load(String instanceId) {
     for (int i = 0; i < _tableUpdaterLocks.length; i++) {
       _tableUpdaterLocks[i] = new Object();
     }
-    SIMPLE_DATE_FORMAT.setTimeZone(TimeZone.getTimeZone("UTC"));
   }
 
   public PinotHelixResourceManager(ControllerConf controllerConf) {
@@ -3603,7 +3604,7 @@ public TableStats getTableStats(String tableNameWithType) {
     String zkPath = ZKMetadataProvider.constructPropertyStorePathForResourceConfig(tableNameWithType);
     Stat stat = _propertyStore.getStat(zkPath, AccessOption.PERSISTENT);
     Preconditions.checkState(stat != null, "Failed to read ZK stats for table: %s", tableNameWithType);
-    String creationTime = SIMPLE_DATE_FORMAT.format(stat.getCtime());
+    String creationTime = SIMPLE_DATE_FORMAT.format(Instant.ofEpochMilli(stat.getCtime()));
     return new TableStats(creationTime);
   }
 

diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/GroovyTemplateUtils.java b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/GroovyTemplateUtils.java
@@ -20,23 +20,22 @@
 
 import groovy.text.SimpleTemplateEngine;
 import java.io.IOException;
-import java.text.SimpleDateFormat;
 import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
 import java.time.temporal.ChronoUnit;
 import java.util.Collections;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.TimeZone;
-
 
 public class GroovyTemplateUtils {
   private GroovyTemplateUtils() {
   }
 
   private static final SimpleTemplateEngine GROOVY_TEMPLATE_ENGINE = new SimpleTemplateEngine();
-  private static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd");
+  private static final DateTimeFormatter DATE_FORMAT =
+      DateTimeFormatter.ofPattern("yyyy-MM-dd").withZone(ZoneOffset.UTC);
 
   public static String renderTemplate(String template, Map<String, Object> newContext)
       throws IOException, ClassNotFoundException {
@@ -53,8 +52,8 @@ public static String renderTemplate(String template, Map<String, Object> newCont
   public static Map<String, Object> getDefaultContextMap() {
     Map<String, Object> defaultContextMap = new HashMap<>();
     Instant now = Instant.now();
-    defaultContextMap.put("today", DATE_FORMAT.format(new Date(now.toEpochMilli())));
-    defaultContextMap.put("yesterday", DATE_FORMAT.format(new Date(now.minus(1, ChronoUnit.DAYS).toEpochMilli())));
+    defaultContextMap.put("today", DATE_FORMAT.format(now));
+    defaultContextMap.put("yesterday", DATE_FORMAT.format(now.minus(1, ChronoUnit.DAYS)));
     return defaultContextMap;
   }
 
@@ -78,7 +77,6 @@ public static String renderTemplate(String template)
   }
 
   static {
-    DATE_FORMAT.setTimeZone(TimeZone.getTimeZone("UTC"));
     GROOVY_TEMPLATE_ENGINE.setEscapeBackslash(true);
   }
 }