diff --git a/docs/changelog/116953.yaml b/docs/changelog/116953.yaml new file mode 100644 index 0000000000000..33616510d8fd0 --- /dev/null +++ b/docs/changelog/116953.yaml @@ -0,0 +1,6 @@ +pr: 116953 +summary: Fix false positive date detection with trailing dot +area: Mapping +type: bug +issues: + - 116946 diff --git a/docs/reference/esql/functions/description/categorize.asciidoc b/docs/reference/esql/functions/description/categorize.asciidoc index a5e8e2d507574..32af0051e91c8 100644 --- a/docs/reference/esql/functions/description/categorize.asciidoc +++ b/docs/reference/esql/functions/description/categorize.asciidoc @@ -3,3 +3,9 @@ *Description* Groups text messages into categories of similarly formatted text values. + +`CATEGORIZE` has the following limitations: + +* can't be used within other expressions +* can't be used with multiple groupings +* can't be used or referenced within aggregations diff --git a/docs/reference/troubleshooting/troubleshooting-unstable-cluster.asciidoc b/docs/reference/troubleshooting/troubleshooting-unstable-cluster.asciidoc index cbb35f7731034..e47b85aa99547 100644 --- a/docs/reference/troubleshooting/troubleshooting-unstable-cluster.asciidoc +++ b/docs/reference/troubleshooting/troubleshooting-unstable-cluster.asciidoc @@ -126,7 +126,7 @@ repeatedly-dropped connections will severely affect its operation. The connections from the elected master node to every other node in the cluster are particularly important. The elected master never spontaneously closes its outbound connections to other nodes. Similarly, once an inbound connection is -fully established, a node never spontaneously it unless the node is shutting +fully established, a node never spontaneously closes it unless the node is shutting down. If you see a node unexpectedly leave the cluster with the `disconnected` diff --git a/muted-tests.yml b/muted-tests.yml index e81ed49f151ff..a09952e878315 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -326,9 +326,6 @@ tests: - class: org.elasticsearch.xpack.searchablesnapshots.RetrySearchIntegTests method: testRetryPointInTime issue: https://github.com/elastic/elasticsearch/issues/117116 -- class: org.elasticsearch.oldrepos.OldRepositoryAccessIT - method: testOldRepoAccess - issue: https://github.com/elastic/elasticsearch/issues/115631 - class: org.elasticsearch.xpack.inference.InferenceRestIT method: test {p0=inference/40_semantic_text_query/Query a field that uses the default ELSER 2 endpoint} issue: https://github.com/elastic/elasticsearch/issues/117027 diff --git a/server/src/main/java/org/elasticsearch/common/time/EpochTime.java b/server/src/main/java/org/elasticsearch/common/time/EpochTime.java index 5693ce50dbe0f..c53c9d0c03df3 100644 --- a/server/src/main/java/org/elasticsearch/common/time/EpochTime.java +++ b/server/src/main/java/org/elasticsearch/common/time/EpochTime.java @@ -246,7 +246,10 @@ public long getFrom(TemporalAccessor temporal) { .toFormatter(Locale.ROOT); // this supports milliseconds ending in dot - private static final DateTimeFormatter MILLISECONDS_FORMATTER2 = new DateTimeFormatterBuilder().append(MILLISECONDS_FORMATTER1) + private static final DateTimeFormatter MILLISECONDS_FORMATTER2 = new DateTimeFormatterBuilder().optionalStart() + .appendText(NEGATIVE_SIGN_FIELD, Map.of(-1L, "-")) // field is only created in the presence of a '-' char. + .optionalEnd() + .appendValue(UNSIGNED_MILLIS, 1, 19, SignStyle.NOT_NEGATIVE) .appendLiteral('.') .toFormatter(Locale.ROOT); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index 1c9321737ab5f..c56885eded38f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -21,6 +21,8 @@ import java.util.List; public class DocumentMapper { + static final NodeFeature INDEX_SORTING_ON_NESTED = new NodeFeature("mapper.index_sorting_on_nested"); + private final String type; private final CompressedXContent mappingSource; private final MappingLookup mappingLookup; @@ -29,8 +31,6 @@ public class DocumentMapper { private final IndexVersion indexVersion; private final Logger logger; - static final NodeFeature INDEX_SORTING_ON_NESTED = new NodeFeature("mapper.index_sorting_on_nested"); - /** * Create a new {@link DocumentMapper} that holds empty mappings. * @param mapperService the mapper service that holds the needed components @@ -72,9 +72,11 @@ public static DocumentMapper createEmpty(MapperService mapperService) { : "provided source [" + source + "] differs from mapping [" + mapping.toCompressedXContent() + "]"; } - private void maybeLogDebug(Exception ex) { + private void maybeLog(Exception ex) { if (logger.isDebugEnabled()) { logger.debug("Error while parsing document: " + ex.getMessage(), ex); + } else if (IntervalThrottler.DOCUMENT_PARSING_FAILURE.accept()) { + logger.error("Error while parsing document: " + ex.getMessage(), ex); } } @@ -125,7 +127,7 @@ public ParsedDocument parse(SourceToParse source) throws DocumentParsingExceptio try { return documentParser.parseDocument(source, mappingLookup); } catch (Exception e) { - maybeLogDebug(e); + maybeLog(e); throw e; } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IntervalThrottler.java b/server/src/main/java/org/elasticsearch/index/mapper/IntervalThrottler.java new file mode 100644 index 0000000000000..ffc35d9eaf769 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/IntervalThrottler.java @@ -0,0 +1,66 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.mapper; + +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Throttles tracked operations based on a time interval, restricting them to 1 per N seconds. + */ +enum IntervalThrottler { + DOCUMENT_PARSING_FAILURE(60); + + static final int MILLISECONDS_IN_SECOND = 1000; + + private final Acceptor acceptor; + + IntervalThrottler(long intervalSeconds) { + acceptor = new Acceptor(intervalSeconds * MILLISECONDS_IN_SECOND); + } + + /** + * @return true if the operation gets accepted, false if throttled. + */ + boolean accept() { + return acceptor.accept(); + } + + // Defined separately for testing. + static class Acceptor { + private final long intervalMillis; + private final AtomicBoolean lastAcceptedGuard = new AtomicBoolean(false); + private volatile long lastAcceptedTimeMillis = 0; + + Acceptor(long intervalMillis) { + this.intervalMillis = intervalMillis; + } + + boolean accept() { + final long now = System.currentTimeMillis(); + // Check without guarding first, to reduce contention. + if (now - lastAcceptedTimeMillis > intervalMillis) { + // Check if another concurrent operation succeeded. + if (lastAcceptedGuard.compareAndSet(false, true)) { + try { + // Repeat check under guard protection, so that only one message gets written per interval. + if (now - lastAcceptedTimeMillis > intervalMillis) { + lastAcceptedTimeMillis = now; + return true; + } + } finally { + // Reset guard. + lastAcceptedGuard.set(false); + } + } + } + return false; + } + } +} diff --git a/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java b/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java index dfe3cf10fd494..ca1ef6cba91e7 100644 --- a/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java +++ b/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java @@ -242,6 +242,17 @@ public void testEpochMillisParser() { assertThat(formatter.format(instant), is("-0.12345")); assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant)); } + { + Instant instant = Instant.from(formatter.parse("12345.")); + assertThat(instant.getEpochSecond(), is(12L)); + assertThat(instant.getNano(), is(345_000_000)); + assertThat(formatter.format(instant), is("12345")); + assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant)); + } + { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> formatter.parse("12345.0.")); + assertThat(e.getMessage(), is("failed to parse date field [12345.0.] with format [epoch_millis]")); + } } /** diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java index db9fdead949de..b2ba3d60d2174 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DocumentMapperTests.java @@ -9,12 +9,18 @@ package org.elasticsearch.index.mapper; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.core.LogEvent; import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.core.KeywordAnalyzer; import org.apache.lucene.analysis.core.WhitespaceAnalyzer; import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.elasticsearch.common.Strings; import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.logging.MockAppender; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.IndexVersion; @@ -493,4 +499,35 @@ public void testDeeplyNestedMapping() throws Exception { } } } + + public void testParsingErrorLogging() throws Exception { + MockAppender appender = new MockAppender("mock_appender"); + appender.start(); + Logger testLogger = LogManager.getLogger(DocumentMapper.class); + Loggers.addAppender(testLogger, appender); + Level originalLogLevel = testLogger.getLevel(); + Loggers.setLevel(testLogger, Level.ERROR); + + try { + DocumentMapper doc = createDocumentMapper(mapping(b -> b.startObject("value").field("type", "integer").endObject())); + + DocumentParsingException e = expectThrows( + DocumentParsingException.class, + () -> doc.parse(source(b -> b.field("value", "foo"))) + ); + assertThat(e.getMessage(), containsString("failed to parse field [value] of type [integer] in document with id '1'")); + LogEvent event = appender.getLastEventAndReset(); + if (event != null) { + assertThat(event.getMessage().getFormattedMessage(), containsString(e.getMessage())); + } + + e = expectThrows(DocumentParsingException.class, () -> doc.parse(source(b -> b.field("value", "foo")))); + assertThat(e.getMessage(), containsString("failed to parse field [value] of type [integer] in document with id '1'")); + assertThat(appender.getLastEventAndReset(), nullValue()); + } finally { + Loggers.setLevel(testLogger, originalLogLevel); + Loggers.removeAppender(testLogger, appender); + appender.stop(); + } + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java index 5df2503a31c1a..3968498b71b3b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java @@ -597,6 +597,23 @@ private void doTestDefaultFloatingPointMappings(DocumentMapper mapper, XContentB assertThat(((FieldMapper) update.getRoot().getMapper("quux")).fieldType().typeName(), equalTo("float")); } + public void testDateDetectionEnabled() throws Exception { + MapperService mapperService = createMapperService(topMapping(b -> b.field("date_detection", true))); + + ParsedDocument doc = mapperService.documentMapper().parse(source(b -> { + b.field("date", "2024-11-18"); + b.field("no_date", "128.0."); + })); + assertNotNull(doc.dynamicMappingsUpdate()); + merge(mapperService, dynamicMapping(doc.dynamicMappingsUpdate())); + + Mapper mapper = mapperService.documentMapper().mappers().getMapper("date"); + assertThat(mapper.typeName(), equalTo("date")); + + mapper = mapperService.documentMapper().mappers().getMapper("no_date"); + assertThat(mapper.typeName(), equalTo("text")); + } + public void testNumericDetectionEnabled() throws Exception { MapperService mapperService = createMapperService(topMapping(b -> b.field("numeric_detection", true))); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IntervalThrottlerTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IntervalThrottlerTests.java new file mode 100644 index 0000000000000..25fd614524441 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/mapper/IntervalThrottlerTests.java @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.mapper; + +import org.elasticsearch.test.ESTestCase; + +public class IntervalThrottlerTests extends ESTestCase { + + public void testThrottling() throws Exception { + var throttler = new IntervalThrottler.Acceptor(10); + assertTrue(throttler.accept()); + assertFalse(throttler.accept()); + assertFalse(throttler.accept()); + + Thread.sleep(20); + assertTrue(throttler.accept()); + assertFalse(throttler.accept()); + assertFalse(throttler.accept()); + } +} diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java index 40703e7b6bada..359fbdf7d0220 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java @@ -26,6 +26,8 @@ import org.apache.http.ssl.SSLContextBuilder; import org.apache.http.ssl.SSLContexts; import org.apache.http.util.EntityUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.Build; import org.elasticsearch.TransportVersion; import org.elasticsearch.TransportVersions; @@ -158,6 +160,8 @@ public abstract class ESRestTestCase extends ESTestCase { private static final Pattern SEMANTIC_VERSION_PATTERN = Pattern.compile("^(\\d+\\.\\d+\\.\\d+)\\D?.*"); + private static final Logger SUITE_LOGGER = LogManager.getLogger(ESRestTestCase.class); + /** * Convert the entity from a {@link Response} into a map of maps. * Consumes the underlying HttpEntity, releasing any resources it may be holding. @@ -1171,7 +1175,13 @@ protected static void wipeAllIndices(boolean preserveSecurityIndices) throws IOE } final Request deleteRequest = new Request("DELETE", Strings.collectionToCommaDelimitedString(indexPatterns)); deleteRequest.addParameter("expand_wildcards", "open,closed" + (includeHidden ? ",hidden" : "")); - deleteRequest.setOptions(deleteRequest.getOptions().toBuilder().setWarningsHandler(ignoreAsyncSearchWarning()).build()); + + // If system index warning, ignore but log + // See: https://github.com/elastic/elasticsearch/issues/117099 + // and: https://github.com/elastic/elasticsearch/issues/115809 + deleteRequest.setOptions( + RequestOptions.DEFAULT.toBuilder().setWarningsHandler(ESRestTestCase::ignoreSystemIndexAccessWarnings) + ); final Response response = adminClient().performRequest(deleteRequest); try (InputStream is = response.getEntity().getContent()) { assertTrue((boolean) XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true).get("acknowledged")); @@ -1184,28 +1194,16 @@ protected static void wipeAllIndices(boolean preserveSecurityIndices) throws IOE } } - // Make warnings handler that ignores the .async-search warning since .async-search may randomly appear when async requests are slow - // See: https://github.com/elastic/elasticsearch/issues/117099 - protected static WarningsHandler ignoreAsyncSearchWarning() { - return new WarningsHandler() { - @Override - public boolean warningsShouldFailRequest(List warnings) { - if (warnings.isEmpty()) { - return false; - } - return warnings.equals( - List.of( - "this request accesses system indices: [.async-search], " - + "but in a future major version, direct access to system indices will be prevented by default" - ) - ) == false; + private static boolean ignoreSystemIndexAccessWarnings(List warnings) { + for (String warning : warnings) { + if (warning.startsWith("this request accesses system indices:")) { + SUITE_LOGGER.warn("Ignoring system index access warning during test cleanup: {}", warning); + } else { + return true; } + } - @Override - public String toString() { - return "ignore .async-search warning"; - } - }; + return false; } protected static void wipeDataStreams() throws IOException { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/Categorize.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/Categorize.java index ca0447ce11ec4..e2c04ecb15b59 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/Categorize.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/Categorize.java @@ -48,6 +48,12 @@ public class Categorize extends GroupingFunction implements Validatable { @FunctionInfo( returnType = "keyword", description = "Groups text messages into categories of similarly formatted text values.", + detailedDescription = """ + `CATEGORIZE` has the following limitations: + + * can't be used within other expressions + * can't be used with multiple groupings + * can't be used or referenced within aggregate functions""", examples = { @Example( file = "docs",