Added performance improvement for datetime field parsing (opensearch-…

…project#9567) * Added performance improvement for datetime field parsing This adds caching of formatters in case of no explicit format specified for the datetime field in mapping. This also adds `strict_date_time_no_millis` as additional formatter in default date time formats Signed-off-by: Prabhat Sharma <[email protected]> * Refactor DateTimeFormatter Access under featireflag Signed-off-by: Prabhat Sharma <[email protected]> --------- Signed-off-by: Prabhat Sharma <[email protected]> Co-authored-by: Prabhat Sharma <[email protected]> (cherry picked from commit 2965e69) Signed-off-by: Prabhat Sharma <[email protected]>
CaptainDredge · Jan 31, 2024 · 70820c5 · 70820c5
1 parent ff820ed
commit 70820c5
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -126,6 +126,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Ensure Jackson default maximums introduced in 2.16.0 do not conflict with OpenSearch settings ([#11890](https://github.com/opensearch-project/OpenSearch/pull/11890))
 - Extract cluster management for integration tests into JUnit test rule out of OpenSearchIntegTestCase ([#11877](https://github.com/opensearch-project/OpenSearch/pull/11877)), ([#12000](https://github.com/opensearch-project/OpenSearch/pull/12000))
 - Workaround for https://bugs.openjdk.org/browse/JDK-8323659 regression, introduced in JDK-21.0.2 ([#11968](https://github.com/opensearch-project/OpenSearch/pull/11968))
+- Performance improvement for Datetime field caching ([#4558](https://github.com/opensearch-project/OpenSearch/issues/4558))
 
 ### Deprecated
 

diff --git a/server/src/main/java/org/opensearch/common/time/JavaDateFormatter.java b/server/src/main/java/org/opensearch/common/time/JavaDateFormatter.java
@@ -52,6 +52,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 
@@ -150,7 +151,7 @@ JavaDateFormatter getRoundupParser() {
         if (parsers.length == 0) {
             this.parsers = Collections.singletonList(printer);
         } else {
-            this.parsers = Arrays.asList(parsers);
+            this.parsers = new CopyOnWriteArrayList<>(parsers);
         }
         List<DateTimeFormatter> roundUp = createRoundUpParser(format, roundupParserConsumer);
         this.roundupParser = new RoundUpFormatter(format, roundUp);
@@ -235,7 +236,17 @@ private JavaDateFormatter(
         this.printFormat = printFormat;
         this.printer = printer;
         this.roundupParser = roundUpParsers != null ? new RoundUpFormatter(format, roundUpParsers) : null;
-        this.parsers = parsers;
+        this.parsers = new CopyOnWriteArrayList<>(parsers);
+        this.canCacheLastParsedFormatter = canCacheLastParsedFormatter;
+    }
+
+    private JavaDateFormatter(
+        String format,
+        DateTimeFormatter printer,
+        List<DateTimeFormatter> roundUpParsers,
+        List<DateTimeFormatter> parsers
+    ) {
+        this(format, format, printer, roundUpParsers, parsers, false);
         this.canCacheLastParsedFormatter = canCacheLastParsedFormatter;
     }
 
@@ -317,7 +328,9 @@ public DateFormatter withZone(ZoneId zoneId) {
         if (zoneId.equals(zone())) {
             return this;
         }
-        List<DateTimeFormatter> parsers = this.parsers.stream().map(p -> p.withZone(zoneId)).collect(Collectors.toList());
+        List<DateTimeFormatter> parsers = new CopyOnWriteArrayList<>(
+            this.parsers.stream().map(p -> p.withZone(zoneId)).collect(Collectors.toList())
+        );
         List<DateTimeFormatter> roundUpParsers = this.roundupParser.getParsers()
             .stream()
             .map(p -> p.withZone(zoneId))
@@ -331,7 +344,9 @@ public DateFormatter withLocale(Locale locale) {
         if (locale.equals(locale())) {
             return this;
         }
-        List<DateTimeFormatter> parsers = this.parsers.stream().map(p -> p.withLocale(locale)).collect(Collectors.toList());
+        List<DateTimeFormatter> parsers = new CopyOnWriteArrayList<>(
+            this.parsers.stream().map(p -> p.withLocale(locale)).collect(Collectors.toList())
+        );
         List<DateTimeFormatter> roundUpParsers = this.roundupParser.getParsers()
             .stream()
             .map(p -> p.withLocale(locale))

diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java
@@ -99,6 +99,17 @@ public static boolean isEnabled(Setting<Boolean> featureFlag) {
         }
     }
 
+    public static boolean isEnabled(Setting<Boolean> featureFlag) {
+        if ("true".equalsIgnoreCase(System.getProperty(featureFlag.getKey()))) {
+            // TODO: Remove the if condition once FeatureFlags are only supported via opensearch.yml
+            return true;
+        } else if (settings != null) {
+            return featureFlag.get(settings);
+        } else {
+            return featureFlag.getDefault(Settings.EMPTY);
+        }
+    }
+
     public static final Setting<Boolean> EXTENSIONS_SETTING = Setting.boolSetting(EXTENSIONS, false, Property.NodeScope);
 
     public static final Setting<Boolean> IDENTITY_SETTING = Setting.boolSetting(IDENTITY, false, Property.NodeScope);