Race condition fix for datetime optimization (opensearch-project#10385)

* Race condition fix for datetime optimization Signed-off-by: Prabhat Sharma <[email protected]> * Changed JavaDateTimeFormatter caching of parser from MRU(most recently used) to a simple last used formatter Signed-off-by: Prabhat Sharma <[email protected]> --------- Signed-off-by: Prabhat Sharma <[email protected]> Co-authored-by: Prabhat Sharma <[email protected]> Signed-off-by: Prabhat Sharma <[email protected]>
CaptainDredge · Oct 6, 2023 · ad17b38 · ad17b38
1 parent 5a459ba
commit ad17b38
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 17 deletions.
diff --git a/server/src/main/java/org/opensearch/common/time/JavaDateFormatter.java b/server/src/main/java/org/opensearch/common/time/JavaDateFormatter.java
@@ -74,6 +74,7 @@ class JavaDateFormatter implements DateFormatter {
     private final List<DateTimeFormatter> parsers;
     private final JavaDateFormatter roundupParser;
     private final Boolean canCacheLastParsedFormatter;
+    private volatile DateTimeFormatter lastParsedformatter = null;
 
     /**
      * A round up formatter
@@ -150,7 +151,7 @@ JavaDateFormatter getRoundupParser() {
         if (parsers.length == 0) {
             this.parsers = Collections.singletonList(printer);
         } else {
-            this.parsers = new CopyOnWriteArrayList<>(parsers);
+            this.parsers = Arrays.asList(parsers);
         }
         List<DateTimeFormatter> roundUp = createRoundUpParser(format, roundupParserConsumer);
         this.roundupParser = new RoundUpFormatter(format, roundUp);
@@ -235,7 +236,7 @@ private JavaDateFormatter(
         this.printFormat = printFormat;
         this.printer = printer;
         this.roundupParser = roundUpParsers != null ? new RoundUpFormatter(format, roundUpParsers) : null;
-        this.parsers = new CopyOnWriteArrayList<>(parsers);
+        this.parsers = parsers;
         this.canCacheLastParsedFormatter = canCacheLastParsedFormatter;
     }
 
@@ -286,24 +287,22 @@ public TemporalAccessor parse(String input) {
     private TemporalAccessor doParse(String input) {
         if (parsers.size() > 1) {
             Object object = null;
-            DateTimeFormatter lastParsedformatter = null;
+            if (canCacheLastParsedFormatter && lastParsedformatter != null) {
+                ParsePosition pos = new ParsePosition(0);
+                object = lastParsedformatter.toFormat().parseObject(input, pos);
+                if (parsingSucceeded(object, input, pos)) {
+                    return (TemporalAccessor) object;
+                }
+            }
             for (DateTimeFormatter formatter : parsers) {
                 ParsePosition pos = new ParsePosition(0);
                 object = formatter.toFormat().parseObject(input, pos);
                 if (parsingSucceeded(object, input, pos)) {
                     lastParsedformatter = formatter;
-                    break;
+                    return (TemporalAccessor) object;
                 }
             }
-            if (lastParsedformatter != null) {
-                if (canCacheLastParsedFormatter && lastParsedformatter != parsers.get(0)) {
-                    synchronized (parsers) {
-                        parsers.remove(lastParsedformatter);
-                        parsers.add(0, lastParsedformatter);
-                    }
-                }
-                return (TemporalAccessor) object;
-            }
+
             throw new DateTimeParseException("Failed to parse with all enclosed parsers", input, 0);
         }
         return this.parsers.get(0).parse(input);

diff --git a/server/src/main/java/org/opensearch/search/DocValueFormat.java b/server/src/main/java/org/opensearch/search/DocValueFormat.java
@@ -35,6 +35,7 @@
 import org.apache.lucene.document.InetAddressPoint;
 import org.apache.lucene.util.BytesRef;
 import org.opensearch.LegacyESVersion;
+import org.opensearch.Version;
 import org.opensearch.common.Numbers;
 import org.opensearch.common.joda.Joda;
 import org.opensearch.common.joda.JodaDateFormatter;
@@ -248,9 +249,9 @@ public DateTime(DateFormatter formatter, ZoneId timeZone, DateFieldMapper.Resolu
         public DateTime(StreamInput in) throws IOException {
             String datePattern = in.readString();
             String printPattern = null;
-            if (in.getVersion().onOrAfter(Version.V_2_1_1)) {
+            if (in.getVersion().onOrAfter(Version.V_2_11_0)) {
                 printPattern = in.readOptionalString();
-
+            }
             String zoneId = in.readString();
             if (in.getVersion().before(LegacyESVersion.V_7_0_0)) {
                 this.timeZone = DateUtils.of(zoneId);
@@ -287,12 +288,12 @@ public String getWriteableName() {
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
-            if(out.getVersion().before(Version.V_2_1_1) && formatter.equals(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER)) {
+            if (out.getVersion().before(Version.V_2_11_0) && formatter.equals(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER)) {
                 out.writeString(DateFieldMapper.LEGACY_DEFAULT_DATE_TIME_FORMATTER.pattern()); // required for backwards compatibility
             } else {
                 out.writeString(formatter.pattern());
             }
-            if (out.getVersion().onOrAfter(Version.V_2_1_1)) {
+            if (out.getVersion().onOrAfter(Version.V_2_11_0)) {
                 out.writeOptionalString(formatter.printPattern());
             }
             if (out.getVersion().before(LegacyESVersion.V_7_0_0)) {