From fbddd58b4fc621b3d384efdf99e5fbb5e5ca8137 Mon Sep 17 00:00:00 2001 From: Prabhat <20185657+CaptainDredge@users.noreply.github.com> Date: Thu, 5 Oct 2023 12:56:07 -0700 Subject: [PATCH] Race condition fix for datetime optimization (#10385) * Race condition fix for datetime optimization Signed-off-by: Prabhat Sharma * Changed JavaDateTimeFormatter caching of parser from MRU(most recently used) to a simple last used formatter Signed-off-by: Prabhat Sharma --------- Signed-off-by: Prabhat Sharma Co-authored-by: Prabhat Sharma Signed-off-by: Prabhat Sharma (cherry picked from commit ad17b3858501a0decbcf6887f4226f8c5db0a534) Signed-off-by: Prabhat Sharma --- .../common/time/JavaDateFormatter.java | 25 +++++++++---------- .../org/opensearch/search/DocValueFormat.java | 9 ++++--- 2 files changed, 17 insertions(+), 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 index 594ebc3b26a73..14191357fb8aa 100644 --- 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 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 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 index aaa14d5cba7f8..b8daa206fa0e9 100644 --- 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)) {