TSDB: add index timestamp range check #78291

weizijun · 2021-09-24T16:47:17Z

Add a index.time_series.start_time and index.time_series.end_time setting to time series indices and reject documents with @timestamp outside the range
Reject documents without an @timestamp

elasticmachine · 2021-09-24T20:13:48Z

Pinging @elastic/es-analytics-geo (Team:Analytics)

nik9000

Ah. You already did the end time only getting bigger. It's just the down time.

Could you make sure the setting aren't set-able without being in time series mode? I think right now they are behind the feature flag but not behind enabling the mode. I probably didn't mention that when we typed at each other earlier.

weizijun · 2021-09-26T11:45:15Z

Could you make sure the setting aren't set-able without being in time series mode? I think right now they are behind the feature flag but not behind enabling the mode. I probably didn't mention that when we typed at each other earlier.

ok, I will add the time_series mode check

nik9000 · 2021-09-26T12:41:16Z

Thanks!

…

On Sun, Sep 26, 2021, 07:45 weizijun ***@***.***> wrote: Could you make sure the setting aren't set-able without being in time series mode? I think right now they are behind the feature flag but not behind enabling the mode. I probably didn't mention that when we typed at each other earlier. ok, I will add the time_series mode check — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#78291 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABUXIWO5LTNGPVRDQH2UG3UD4BVNANCNFSM5EWKXGYA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

server/src/main/java/org/elasticsearch/index/IndexMode.java

server/src/main/java/org/elasticsearch/index/IndexSettings.java

server/src/main/java/org/elasticsearch/index/IndexMode.java

server/src/main/java/org/elasticsearch/index/IndexSettings.java

server/src/test/java/org/elasticsearch/index/IndexSettingsTests.java

server/src/main/java/org/elasticsearch/index/IndexSettings.java

imotov

@nik9000 let's discuss the timestamp issue.

server/src/main/java/org/elasticsearch/index/IndexSettings.java

server/src/main/java/org/elasticsearch/index/IndexMode.java

This reverts commit b4a68c3.

This reverts commit 63e552c.

* upstream/master: (521 commits) Migrate custom role providers to licensed feature (elastic#79127) Remove stale AwaitsFix in InternalEngineTests (elastic#79323) Fix errors in RefreshListenersTests (elastic#79324) Reeable BwC Tests after elastic#79318 (elastic#79320) Mute BwC Tests for elastic#79318 (elastic#79319) Reenable BwC Tests after elastic#79308 (elastic#79313) Disable BwC Tests for elastic#79308 (elastic#79310) Adjust BWC for node-level field cap requests (elastic#79301) Allow total memory to be overridden (elastic#78750) Fix SnapshotBasedIndexRecoveryIT#testRecoveryIsCancelledAfterDeletingTheIndex (elastic#79269) Disable BWC tests Mute GeoIpDownloaderCliIT.testStartWithNoDatabases (elastic#79299) Add alias support to fleet search API (elastic#79285) Create a coordinating node level reader for tsdb (elastic#79197) Route documents to the correct shards in tsdb (elastic#77731) Inject migrate action regardless of allocate action (elastic#79090) Migrate to data tiers should always ensure a TIER_PREFERENCE is set (elastic#79100) Skip building of BWC distributions when building release artifacts (elastic#79180) Default ENFORCE_DEFAULT_TIER_PREFERENCE to true (elastic#79275) Deprecation of transient cluster settings (elastic#78794) ... # Conflicts: # rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/10_settings.yml # server/src/main/java/org/elasticsearch/common/settings/IndexScopedSettings.java # server/src/main/java/org/elasticsearch/common/settings/Setting.java # server/src/main/java/org/elasticsearch/index/IndexMode.java # server/src/test/java/org/elasticsearch/index/TimeSeriesModeTests.java

imotov · 2021-10-18T20:03:37Z

@elasticmachine test this please

weizijun · 2021-10-19T03:28:22Z

@elasticmachine test this please

@imotov After #79136 Merged, I will change the timestamp check in DataStreamTimestampFieldMapper

imotov · 2021-10-19T03:35:19Z

There are test failing here. I am not sure if you can see them.

org.elasticsearch.xpack.ccr.action.TransportResumeFollowActionTests > testDynamicIndexSettingsAreClassified FAILED
    java.lang.AssertionError: setting [index.time_series.end_time] is not classified as replicated or not replicated
    Expected: is <true>
         but: was <false>
        at __randomizedtesting.SeedInfo.seed([9AB92C831026ACBD:2F05BBEA3A1C9FC7]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.junit.Assert.assertThat(Assert.java:956)
        at org.elasticsearch.xpack.ccr.action.TransportResumeFollowActionTests.testDynamicIndexSettingsAreClassified(TransportResumeFollowActionTests.java:227)
REPRODUCE WITH: ./gradlew ':x-pack:plugin:ccr:test' --tests "org.elasticsearch.xpack.ccr.action.TransportResumeFollowActionTests.testDynamicIndexSettingsAreClassified" -Dtests.seed=9AB92C831026ACBD -Dtests.locale=ar-EG -Dtests.timezone=Africa/Windhoek -Druntime.java=11

Suite: Test class org.elasticsearch.xpack.ccr.action.TransportResumeFollowActionTests
  1> [2021-10-18T22:07:28,045][INFO ][o.e.x.c.a.TransportResumeFollowActionTests] [testDynamicIndexSettingsAreClassified] before test
  1> [2021-10-18T22:07:28,568][INFO ][o.e.x.c.a.TransportResumeFollowActionTests] [testDynamicIndexSettingsAreClassified] after test
  2> REPRODUCE WITH: ./gradlew ':x-pack:plugin:ccr:test' --tests "org.elasticsearch.xpack.ccr.action.TransportResumeFollowActionTests.testDynamicIndexSettingsAreClassified" -Dtests.seed=9AB92C831026ACBD -Dtests.locale=ar-EG -Dtests.timezone=Africa/Windhoek -Druntime.java=11
  2> java.lang.AssertionError: setting [index.time_series.end_time] is not classified as replicated or not replicated
    Expected: is <true>
         but: was <false>
        at __randomizedtesting.SeedInfo.seed([9AB92C831026ACBD:2F05BBEA3A1C9FC7]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.junit.Assert.assertThat(Assert.java:956)
        at org.elasticsearch.xpack.ccr.action.TransportResumeFollowActionTests.testDynamicIndexSettingsAreClassified(TransportResumeFollowActionTests.java:227)
  1> [2021-10-18T22:07:28,617][INFO ][o.e.x.c.a.TransportResumeFollowActionTests] [testValidation] before test
  1> [2021-10-18T22:07:29,608][INFO ][o.e.x.c.a.TransportResumeFollowActionTests] [testValidation] after test
  1> [2021-10-18T22:07:29,631][INFO ][o.e.x.c.a.TransportResumeFollowActionTests] [testFilter] before test
  1> [2021-10-18T22:07:29,632][INFO ][o.e.x.c.a.TransportResumeFollowActionTests] [testFilter] after test
  2> NOTE: leaving temporary files on disk at: /dev/shm/elastic+elasticsearch+pull-request+part-2/x-pack/plugin/ccr/build/testrun/test/temp/org.elasticsearch.xpack.ccr.action.TransportResumeFollowActionTests_9AB92C831026ACBD-001
  2> NOTE: test params are: codec=Asserting(Lucene90): {}, docValues:{}, maxPointsInLeafNode=803, maxMBSortInHeap=7.284619573576025, sim=Asserting(RandomSimilarity(queryNorm=false): {}), locale=ar-EG, timezone=Africa/Windhoek
  2> NOTE: Linux 5.10.0-9-cloud-amd64 amd64/Oracle Corporation 11.0.12 (64-bit)/cpus=32,threads=1,free=429254720,total=536870912
  2> NOTE: All tests run in this JVM: [ActivateAutoFollowPatternActionRequestTests, ShardChangesResponseTests, TransportResumeFollowActionTests]
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF8 -Dsun.jnu.encoding=UTF8

org.elasticsearch.index.TimeSeriesModeTests > testValidateTimestamp FAILED
    java.lang.IllegalArgumentException: [index.mode=time_series] requires [index.routing_path]
        at __randomizedtesting.SeedInfo.seed([84AD7654F32C4F66:426B8A67AE365071]:0)
        at org.elasticsearch.index.IndexMode$2.validateWithOtherSettings(IndexMode.java:70)
        at org.elasticsearch.index.IndexSettings$1.validate(IndexSettings.java:356)
        at org.elasticsearch.index.IndexSettings$1.validate(IndexSettings.java:350)
        at org.elasticsearch.common.settings.Setting.get(Setting.java:484)
        at org.elasticsearch.common.settings.Setting.get(Setting.java:455)
        at org.elasticsearch.common.settings.Setting.get(Setting.java:598)
        at org.elasticsearch.common.settings.AbstractScopedSettings.get(AbstractScopedSettings.java:709)
        at org.elasticsearch.index.IndexSettings.<init>(IndexSettings.java:581)
        at org.elasticsearch.index.IndexSettings.<init>(IndexSettings.java:561)
        at org.elasticsearch.index.mapper.MapperServiceTestCase.createIndexSettings(MapperServiceTestCase.java:212)
        at org.elasticsearch.index.mapper.MapperServiceTestCase.createMapperService(MapperServiceTestCase.java:183)
        at org.elasticsearch.index.mapper.MapperServiceTestCase.createMapperService(MapperServiceTestCase.java:171)
        at org.elasticsearch.index.mapper.MapperServiceTestCase.createMapperService(MapperServiceTestCase.java:140)
        at org.elasticsearch.index.TimeSeriesModeTests.testValidateTimestamp(TimeSeriesModeTests.java:83)
REPRODUCE WITH: ./gradlew ':server:test' --tests "org.elasticsearch.index.TimeSeriesModeTests.testValidateTimestamp" -Dtests.seed=84AD7654F32C4F66 -Dtests.locale=nb -Dtests.timezone=America/Marigot -Druntime.java=11

weizijun · 2021-10-19T03:45:07Z

I'm trying to fix it. If I have any questions, I will ask your help.

* upstream/master: (209 commits) Enforce license expiration (elastic#79671) TSDB: Automatically add timestamp mapper (elastic#79136) [DOCS] `_id` is required for bulk API's `update` action (elastic#79774) EQL: Add optional fields and limit joining keys on non-null values only (elastic#79677) [DOCS] Document range enrich policy (elastic#79607) [DOCS] Fix typos in 8.0 security migration (elastic#79802) Allow listing older repositories (elastic#78244) [ML] track inference model feature usage per node (elastic#79752) Remove IncrementalClusterStateWriter & related code (elastic#79738) Reuse previous indices lookup when possible (elastic#79004) Reduce merging in PersistedClusterStateService (elastic#79793) SQL: Adjust JDBC docs to use milliseconds for timeouts (elastic#79628) Remove endpoint for freezing indices (elastic#78918) [ML] add timeout parameter for DELETE trained_models API (elastic#79739) [ML] wait for .ml-state-write alias to be readable (elastic#79731) [Docs] Update edgengram-tokenizer.asciidoc (elastic#79577) [Test][Transform] fix UpdateTransformActionRequestTests failure (elastic#79787) Limit CS Update Task Description Size (elastic#79443) Apply the reader wrapper on can_match source (elastic#78988) [DOCS] Adds new transform limitation item and a note to the tutorial (elastic#79479) ... # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexMode.java # server/src/test/java/org/elasticsearch/index/TimeSeriesModeTests.java

weizijun · 2021-11-02T06:30:09Z

@elasticmachine test this please

@imotov I fixed the bwc failed tests. The reason for the failure is that the latest version is 8.1.0, and the settings skip version is before 8.0.0. I fixed it.

imotov · 2021-11-02T16:18:41Z

@elasticmachine test this please

nik9000 · 2021-11-10T14:45:43Z

server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java

+            if (value >= endTime) {
+                throw new IllegalArgumentException("time series index @timestamp value [" + value + "] must be smaller than " + endTime);
+            }
+        }


I know I wrote this, but on further reading, I think maybe it makes sense to just move it all to DataStreamTimestampFieldMapper. I thought I was being clever, but all of this code should be able to run just fine from there and it'd be nice that DateFieldMapper doesn't need to care about the validation.

I agree with moving this logic to DataStreamTimestampFieldMapper, since there there is already validation for the @timestamp field (whether it exists and whether a single value has been specified). And I think at least initially, tsdb will work only with data streams.

nik9000 · 2021-11-11T14:36:40Z

server/src/main/java/org/elasticsearch/index/IndexSettings.java

+    /**
+     * End time of the time_series index.
+     */
+    private volatile Instant timeSeriesEndTime;


I'm worried that right now these can't be null so all indices are going to validate the range. I wonder if we can convert these to null for the defaults so we disable the validation when it isn't set.

nik9000 · 2021-11-11T15:43:57Z

@elasticmachine, test this please

nik9000 · 2021-11-11T15:51:26Z

@elasticmachine, test this please

nik9000 · 2021-11-11T16:34:50Z

@elasticmachine, test this please

nik9000 · 2021-11-11T16:49:47Z

@elasticmachine, test this please

nik9000 · 2021-11-11T18:20:25Z

All checks have passed

Nice

nik9000 · 2021-11-11T19:01:05Z

Thanks for all that back and for @weizijun ! I've merged master in and merged it.

weizijun added 2 commits September 24, 2021 15:09

add settings

2bad389

add check code

8809757

elasticsearchmachine added v8.0.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Sep 24, 2021

add some comment

6c42357

nik9000 requested review from nik9000 and imotov September 24, 2021 20:13

nik9000 added the :StorageEngine/TSDB You know, for Metrics label Sep 24, 2021

elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Sep 24, 2021

add yaml tests

c3f1e66

nik9000 reviewed Sep 26, 2021

View reviewed changes

add the time_series mode check

79e96e0

nik9000 reviewed Sep 27, 2021

View reviewed changes

imotov reviewed Sep 27, 2021

View reviewed changes

server/src/main/java/org/elasticsearch/index/IndexSettings.java Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/index/IndexMode.java Outdated Show resolved Hide resolved

server/src/main/java/org/elasticsearch/index/IndexMode.java Outdated Show resolved Hide resolved

weizijun added 5 commits September 28, 2021 19:15

improve

b4a68c3

Revert "improve"

63e552c

This reverts commit b4a68c3.

Revert "Revert "improve""

0e67726

This reverts commit 63e552c.

spotless

78c132d

nik9000 mentioned this pull request Oct 26, 2021

Add better support for metric data types (TSDB) #74660

Closed

weizijun added 3 commits October 26, 2021 23:24

improve

120cdaf

fix ccr tests

563ca87

elasticmachine and others added 2 commits November 1, 2021 21:00

Merge branch 'master' into tsdb-deal-timestreamp

d631649

fix bwc

1dcebb0

weizijun and others added 2 commits November 3, 2021 23:41

improve

79db882

Maybe like this?

69df14d

nik9000 reviewed Nov 10, 2021

View reviewed changes

martijnvg self-requested a review November 11, 2021 11:14

weizijun added 2 commits November 11, 2021 21:30

fix failed tests

6c77b18

fix failed tests

40f2e82

nik9000 reviewed Nov 11, 2021

View reviewed changes

weizijun added 4 commits November 11, 2021 22:39

fix failed tests

582588f

improve

bfe25c5

checkStyle

6350ee1

spotless

a5a116f

fixex tsdb settings tests

7236ee1

Merge branch 'master' into tsdb-deal-timestreamp

20c21a8

Fixup after merge

9e92d09

nik9000 merged commit 8b2019a into elastic:master Nov 11, 2021

wchaparro assigned weizijun and unassigned weizijun Dec 16, 2021

sethmlarson mentioned this pull request Dec 28, 2021

Add IndexSettings.time_series field elastic/elasticsearch-specification#1229

Merged

mark-vieira added v8.0.0-rc1 and removed v8.0.0 labels Jan 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TSDB: add index timestamp range check #78291

TSDB: add index timestamp range check #78291

weizijun commented Sep 24, 2021

elasticmachine commented Sep 24, 2021

nik9000 left a comment

weizijun commented Sep 26, 2021

nik9000 commented Sep 26, 2021 via email

imotov left a comment

imotov commented Oct 18, 2021

weizijun commented Oct 19, 2021

imotov commented Oct 19, 2021

weizijun commented Oct 19, 2021

weizijun commented Nov 2, 2021

imotov commented Nov 2, 2021

nik9000 Nov 10, 2021

martijnvg Nov 11, 2021

nik9000 Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

TSDB: add index timestamp range check #78291

TSDB: add index timestamp range check #78291

Conversation

weizijun commented Sep 24, 2021

elasticmachine commented Sep 24, 2021

nik9000 left a comment

Choose a reason for hiding this comment

weizijun commented Sep 26, 2021

nik9000 commented Sep 26, 2021 via email

imotov left a comment

Choose a reason for hiding this comment

imotov commented Oct 18, 2021

weizijun commented Oct 19, 2021

imotov commented Oct 19, 2021

weizijun commented Oct 19, 2021

weizijun commented Nov 2, 2021

imotov commented Nov 2, 2021

nik9000 Nov 10, 2021

Choose a reason for hiding this comment

martijnvg Nov 11, 2021

Choose a reason for hiding this comment

nik9000 Nov 11, 2021

Choose a reason for hiding this comment

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021

nik9000 commented Nov 11, 2021