From a589e29284ff9b42a4f7bfc29d527c68d6d7691c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 22 Jan 2019 12:29:24 -0500 Subject: [PATCH] Remove warn-date from warning headers (#37622) This commit removes the warn-date from warning headers. Previously we were stamping every warning header with when the request occurred. However, this has a severe performance penalty when deprecation logging is called frequently, as obtaining the current time and formatting it properly is expensive. A previous change moved to using the startup time as the time to stamp on every warning header, but this was only to prove that the timestamping was expensive. Since the warn-date is optional, we elect to remove it from the warning header. Prior to this commit, we worked in Kibana to make the warn-date treated as optional there so that we can follow-up in Elasticsearch and remove the warn-date. This commit does that. --- .../org/elasticsearch/client/Response.java | 4 +- .../common/logging/DeprecationLogger.java | 104 ++---------------- 2 files changed, 14 insertions(+), 94 deletions(-) diff --git a/client/rest/src/main/java/org/elasticsearch/client/Response.java b/client/rest/src/main/java/org/elasticsearch/client/Response.java index 4115ef36a5cb4..e267a68bcbd91 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/Response.java +++ b/client/rest/src/main/java/org/elasticsearch/client/Response.java @@ -103,7 +103,7 @@ public HttpEntity getEntity() { private static final Pattern WARNING_HEADER_PATTERN = Pattern.compile( "299 " + // warn code "Elasticsearch-\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-(?:[a-f0-9]{7}|Unknown) " + // warn agent - "\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\" " + // quoted warning value, captured + "\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\"( " + // quoted warning value, captured // quoted RFC 1123 date format "\"" + // opening quote "(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), " + // weekday @@ -112,7 +112,7 @@ public HttpEntity getEntity() { "\\d{4} " + // 4-digit year "\\d{2}:\\d{2}:\\d{2} " + // (two-digit hour):(two-digit minute):(two-digit second) "GMT" + // GMT - "\""); // closing quote + "\")?"); // closing quote (optional, since an older version can still send a warn-date) /** * Returns a list of all warning headers returned in the response. diff --git a/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java b/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java index af03a0fad03dd..e7118487d3b54 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java +++ b/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java @@ -27,14 +27,8 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import java.nio.charset.Charset; -import java.time.ZoneId; -import java.time.ZonedDateTime; -import java.time.format.DateTimeFormatter; -import java.time.format.DateTimeFormatterBuilder; -import java.time.format.SignStyle; import java.util.BitSet; import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Locale; @@ -45,14 +39,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import static java.time.temporal.ChronoField.DAY_OF_MONTH; -import static java.time.temporal.ChronoField.DAY_OF_WEEK; -import static java.time.temporal.ChronoField.HOUR_OF_DAY; -import static java.time.temporal.ChronoField.MINUTE_OF_HOUR; -import static java.time.temporal.ChronoField.MONTH_OF_YEAR; -import static java.time.temporal.ChronoField.SECOND_OF_MINUTE; -import static java.time.temporal.ChronoField.YEAR; - /** * A logger that logs deprecation notices. */ @@ -163,73 +149,14 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "", Build.CURRENT.shortHash()); - /* - * RFC 7234 section 5.5 specifies that the warn-date is a quoted HTTP-date. HTTP-date is defined in RFC 7234 Appendix B as being from - * RFC 7231 section 7.1.1.1. RFC 7231 specifies an HTTP-date as an IMF-fixdate (or an obs-date referring to obsolete formats). The - * grammar for IMF-fixdate is specified as 'day-name "," SP date1 SP time-of-day SP GMT'. Here, day-name is - * (Mon|Tue|Wed|Thu|Fri|Sat|Sun). Then, date1 is 'day SP month SP year' where day is 2DIGIT, month is - * (Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec), and year is 4DIGIT. Lastly, time-of-day is 'hour ":" minute ":" second' where - * hour is 2DIGIT, minute is 2DIGIT, and second is 2DIGIT. Finally, 2DIGIT and 4DIGIT have the obvious definitions. - */ - private static final DateTimeFormatter RFC_7231_DATE_TIME; - - static { - final Map dow = new HashMap<>(); - dow.put(1L, "Mon"); - dow.put(2L, "Tue"); - dow.put(3L, "Wed"); - dow.put(4L, "Thu"); - dow.put(5L, "Fri"); - dow.put(6L, "Sat"); - dow.put(7L, "Sun"); - final Map moy = new HashMap<>(); - moy.put(1L, "Jan"); - moy.put(2L, "Feb"); - moy.put(3L, "Mar"); - moy.put(4L, "Apr"); - moy.put(5L, "May"); - moy.put(6L, "Jun"); - moy.put(7L, "Jul"); - moy.put(8L, "Aug"); - moy.put(9L, "Sep"); - moy.put(10L, "Oct"); - moy.put(11L, "Nov"); - moy.put(12L, "Dec"); - RFC_7231_DATE_TIME = new DateTimeFormatterBuilder() - .parseCaseInsensitive() - .parseLenient() - .optionalStart() - .appendText(DAY_OF_WEEK, dow) - .appendLiteral(", ") - .optionalEnd() - .appendValue(DAY_OF_MONTH, 2, 2, SignStyle.NOT_NEGATIVE) - .appendLiteral(' ') - .appendText(MONTH_OF_YEAR, moy) - .appendLiteral(' ') - .appendValue(YEAR, 4) - .appendLiteral(' ') - .appendValue(HOUR_OF_DAY, 2) - .appendLiteral(':') - .appendValue(MINUTE_OF_HOUR, 2) - .optionalStart() - .appendLiteral(':') - .appendValue(SECOND_OF_MINUTE, 2) - .optionalEnd() - .appendLiteral(' ') - .appendOffset("+HHMM", "GMT") - .toFormatter(Locale.getDefault(Locale.Category.FORMAT)); - } - - private static final String STARTUP_TIME = RFC_7231_DATE_TIME.format(ZonedDateTime.now(ZoneId.of("GMT"))); - /** * Regular expression to test if a string matches the RFC7234 specification for warning headers. This pattern assumes that the warn code * is always 299. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build hash. */ - public static Pattern WARNING_HEADER_PATTERN = Pattern.compile( + public static final Pattern WARNING_HEADER_PATTERN = Pattern.compile( "299 " + // warn code "Elasticsearch-\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-(?:[a-f0-9]{7}|Unknown) " + // warn agent - "\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\" " + // quoted warning value, captured + "\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\"( " + // quoted warning value, captured // quoted RFC 1123 date format "\"" + // opening quote "(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), " + // weekday @@ -238,12 +165,11 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje "\\d{4} " + // 4-digit year "\\d{2}:\\d{2}:\\d{2} " + // (two-digit hour):(two-digit minute):(two-digit second) "GMT" + // GMT - "\""); // closing quote + "\")?"); // closing quote (optional, since an older version can still send a warn-date) /** * Extracts the warning value from the value of a warning header that is formatted according to RFC 7234. That is, given a string - * {@code 299 Elasticsearch-6.0.0 "warning value" "Sat, 25 Feb 2017 10:27:43 GMT"}, the return value of this method would be {@code - * warning value}. + * {@code 299 Elasticsearch-6.0.0 "warning value"}, the return value of this method would be {@code warning value}. * * @param s the value of a warning header formatted according to RFC 7234. * @return the extracted warning value @@ -251,23 +177,19 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje public static String extractWarningValueFromWarningHeader(final String s) { /* * We know the exact format of the warning header, so to extract the warning value we can skip forward from the front to the first - * quote, and skip backwards from the end to the penultimate quote: - * - * 299 Elasticsearch-6.0.0 "warning value" "Sat, 25, Feb 2017 10:27:43 GMT" - * ^ ^ ^ - * firstQuote penultimateQuote lastQuote + * quote and we know the last quote is at the end of the string * - * We do it this way rather than seeking forward after the first quote because there could be escaped quotes in the warning value - * but since there are none in the warning date, we can skip backwards to find the quote that closes the quoted warning value. + * 299 Elasticsearch-6.0.0 "warning value" + * ^ ^ + * firstQuote lastQuote * * We parse this manually rather than using the capturing regular expression because the regular expression involves a lot of * backtracking and carries a performance penalty. However, when assertions are enabled, we still use the regular expression to * verify that we are maintaining the warning header format. */ final int firstQuote = s.indexOf('\"'); - final int lastQuote = s.lastIndexOf('\"'); - final int penultimateQuote = s.lastIndexOf('\"', lastQuote - 1); - final String warningValue = s.substring(firstQuote + 1, penultimateQuote - 2); + final int lastQuote = s.length() - 1; + final String warningValue = s.substring(firstQuote + 1, lastQuote); assert assertWarningValue(s, warningValue); return warningValue; } @@ -329,9 +251,7 @@ void deprecated(final Set threadContexts, final String message, f * @return a warning value formatted according to RFC 7234 */ public static String formatWarning(final String s) { - return WARNING_PREFIX + " " - + "\"" + escapeAndEncode(s) + "\"" + " " - + "\"" + STARTUP_TIME + "\""; + return WARNING_PREFIX + " " + "\"" + escapeAndEncode(s) + "\""; } /** @@ -442,7 +362,7 @@ static String encode(final String s) { int startIndex = i; do { i++; - } while (i < s.length() && !doesNotNeedEncoding.get(s.charAt(i))); + } while (i < s.length() && doesNotNeedEncoding.get(s.charAt(i)) == false); final byte[] bytes = s.substring(startIndex, i).getBytes(UTF_8); // noinspection ForLoopReplaceableByForEach