Mitigate date histogram slowdowns with non-fixed timezones. #30534

jpountz · 2018-05-11T13:03:28Z

Date histograms on non-fixed timezones such as Europe/Paris proved much slower
than histograms on fixed timezones in #28727. This change mitigates the issue by
using a fixed time zone instead when shard data doesn't cross a transition so
that all timestamps share the same fixed offset. This should be a common case
with daily indices.

NOTE: Rewriting the aggregation doesn't work since the timezone is then also
used on the coordinating node to create empty buckets, which might be out of the
range of data that exists on the shard.

NOTE: In order to be able to get a shard context in the tests, I reused code
from the base query test case by creating a new parent test case for both
queries and aggregations: AbstractBuilderTestCase.

Mitigates #28727

Date histograms on non-fixed timezones such as `Europe/Paris` proved much slower than histograms on fixed timezones in elastic#28727. This change mitigates the issue by using a fixed time zone instead when shard data doesn't cross a transition so that all timestamps share the same fixed offset. This should be a common case with daily indices. NOTE: Rewriting the aggregation doesn't work since the timezone is then also used on the coordinating node to create empty buckets, which might be out of the range of data that exists on the shard. NOTE: In order to be able to get a shard context in the tests, I reused code from the base query test case by creating a new parent test case for both queries and aggregations: `AbstractBuilderTestCase`. Mitigates elastic#28727

elasticmachine · 2018-05-11T13:03:29Z

Pinging @elastic/es-search-aggs

colings86 · 2018-05-11T13:07:08Z

.../org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java

+            if (ft != null && reader != null) {
+                Long anyInstant = null;
+                final IndexFieldData<?> fieldData = context.getForField(ft);
+                if (fieldData instanceof IndexNumericFieldData) {


Should this not be guaranteed to be an IndexNumericFieldData? if so maybe this should either be an assert or we should throw an exception if this condition is false?

good point. I had it implemented in rewrite() initially where it felt wrong since validation happens afterwards, but here the values source is already resolved so an exception would have been thrown already if the histogram ran against a non-numeric field

colings86

I left a couple of comments

colings86 · 2018-05-11T13:11:58Z

...src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramTests.java

+                    assertSame(tz, builder.rewriteTimeZone(shardContextThatCrosses));
+                }
+            }
+        }


Should we also have a test that makes sure fixed time zones are not changed?

the above assertions check it?

sorry I had missed that, you are right

colings86

LGTM

jpountz · 2018-05-14T13:36:30Z

@colings86 I had to do some changes because of test failures in case all values were between the same transitions, but some rounded values were not. Could you have another look?

colings86

@jpountz good catch on the case where the start date and transition are in the same bucket. The solution LGTM

Date histograms on non-fixed timezones such as `Europe/Paris` proved much slower than histograms on fixed timezones in #28727. This change mitigates the issue by using a fixed time zone instead when shard data doesn't cross a transition so that all timestamps share the same fixed offset. This should be a common case with daily indices. NOTE: Rewriting the aggregation doesn't work since the timezone is then also used on the coordinating node to create empty buckets, which might be out of the range of data that exists on the shard. NOTE: In order to be able to get a shard context in the tests, I reused code from the base query test case by creating a new parent test case for both queries and aggregations: `AbstractBuilderTestCase`. Mitigates #28727

…ngs-to-true * elastic/master: (34 commits) Test: increase search logging for LicensingTests Adjust serialization version in IndicesOptions [TEST] Fix compilation Remove version argument in RangeFieldType (elastic#30411) Remove unused DirectoryUtils class. (elastic#30582) Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534) Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594) Removes AwaitsFix on IndicesOptionsTests Template upgrades should happen in a system context (elastic#30621) Fix bug in BucketMetrics path traversal (elastic#30632) Fixes IndiceOptionsTests to serialise correctly (elastic#30644) S3 repo plugin populate SettingsFilter (elastic#30652) mute IndicesOptionsTests.testSerialization Rest High Level client: Add List Tasks (elastic#29546) Refactors ClientHelper to combine header logic (elastic#30620) [ML] Wait for ML indices in rolling upgrade tests (elastic#30615) Watcher: Ensure secrets integration tests also run triggered watch (elastic#30478) Move allocation awareness attributes to list setting (elastic#30626) [Docs] Update code snippet in has-child-query.asciidoc (elastic#30510) Replace custom reloadable Key/TrustManager (elastic#30509) ...

* es/master: (74 commits) Preserve REST client auth despite 401 response (#30558) [test] packaging: add windows boxes (#30402) Make xpack modules instead of a meta plugin (#30589) Mute ShrinkIndexIT [ML] DeleteExpiredDataAction should use client with origin (#30646) Reindex: Fixed typo in assertion failure message (#30619) [DOCS] Fixes list of unconverted snippets in build.gradle [DOCS] Reorganizes RBAC documentation SQL: Remove dependency for server's version from JDBC driver (#30631) Test: increase search logging for LicensingTests Adjust serialization version in IndicesOptions [TEST] Fix compilation Remove version argument in RangeFieldType (#30411) Remove unused DirectoryUtils class. (#30582) Mitigate date histogram slowdowns with non-fixed timezones. (#30534) Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (#29594) Removes AwaitsFix on IndicesOptionsTests Template upgrades should happen in a system context (#30621) Fix bug in BucketMetrics path traversal (#30632) Fixes IndiceOptionsTests to serialise correctly (#30644) ...

* es/6.x: (44 commits) SQL: Remove dependency for server's version from JDBC driver (#30631) Make xpack modules instead of a meta plugin (#30589) Security: Remove SecurityLifecycleService (#30526) Build: Add task interdependencies for ssl configuration (#30633) Mute ShrinkIndexIT [ML] DeleteExpiredDataAction should use client with origin (#30646) Reindex: Fixed typo in assertion failure message (#30619) [DOCS] Fixes list of unconverted snippets in build.gradle Use readFully() to read bytes from CipherInputStream (#30640) Add Create Repository High Level REST API (#30501) [DOCS] Reorganizes RBAC documentation Test: increase search logging for LicensingTests Delay _uid field data deprecation warning (#30651) Deprecate Empty Templates (#30194) Remove unused DirectoryUtils class. (#30582) Mitigate date histogram slowdowns with non-fixed timezones. (#30534) [TEST] Remove AwaitsFix in IndicesOptionsTests#testSerialization S3 repo plugin populates SettingsFilter (#30652) Rest High Level client: Add List Tasks (#29546) Fixes IndiceOptionsTests to serialise correctly (#30644) ...

* es/ccr: (75 commits) Preserve REST client auth despite 401 response (elastic#30558) [test] packaging: add windows boxes (elastic#30402) Make xpack modules instead of a meta plugin (elastic#30589) Mute ShrinkIndexIT [ML] DeleteExpiredDataAction should use client with origin (elastic#30646) Reindex: Fixed typo in assertion failure message (elastic#30619) [DOCS] Fixes list of unconverted snippets in build.gradle [DOCS] Reorganizes RBAC documentation SQL: Remove dependency for server's version from JDBC driver (elastic#30631) Test: increase search logging for LicensingTests Adjust serialization version in IndicesOptions [TEST] Fix compilation Remove version argument in RangeFieldType (elastic#30411) Remove unused DirectoryUtils class. (elastic#30582) Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534) Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594) Removes AwaitsFix on IndicesOptionsTests Template upgrades should happen in a system context (elastic#30621) Fix bug in BucketMetrics path traversal (elastic#30632) Fixes IndiceOptionsTests to serialise correctly (elastic#30644) ...

…30534) Date histograms on non-fixed timezones such as `Europe/Paris` proved much slower than histograms on fixed timezones in elastic#28727. This change mitigates the issue by using a fixed time zone instead when shard data doesn't cross a transition so that all timestamps share the same fixed offset. This should be a common case with daily indices. NOTE: Rewriting the aggregation doesn't work since the timezone is then also used on the coordinating node to create empty buckets, which might be out of the range of data that exists on the shard. NOTE: In order to be able to get a shard context in the tests, I reused code from the base query test case by creating a new parent test case for both queries and aggregations: `AbstractBuilderTestCase`. Mitigates elastic#28727

jpountz added >enhancement :Analytics/Aggregations Aggregations v7.0.0 v6.4.0 labels May 11, 2018

colings86 reviewed May 11, 2018

View reviewed changes

No need for instanceof.

da06498

colings86 approved these changes May 11, 2018

View reviewed changes

jpountz added 3 commits May 14, 2018 09:50

Fix failure.

9646c0b

iter

fbd1ad6

Merge branch 'master' into enhancement/time_zone_rounding

44b8820

colings86 approved these changes May 15, 2018

View reviewed changes

jpountz mentioned this pull request May 15, 2018

Date histograms can be very slow due to time_zone elastic/kibana#18853

Closed

jpountz merged commit 28d4685 into elastic:master May 16, 2018

jpountz deleted the enhancement/time_zone_rounding branch May 16, 2018 15:06

$@polyfractal$ polyfractal mentioned this pull request Jun 4, 2018

time_zone option makes date histograms much slower #28727

Closed

This was referenced Jun 18, 2018

Unable to open index in Discover and Visualize when date format is epoch_millis elastic/kibana#19858

Closed

Date field mapping "epoch_millis" errors on date aggregations with timezones other than UTC #31392

Closed

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

jimczi mentioned this pull request Sep 12, 2019

Slow date_histogram after upgrading to 7.3.0 #45702

Closed

DaveCTurner mentioned this pull request Sep 17, 2019

TimeIntervalRounding is not optimised for fixed-offset timezones #46771

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mitigate date histogram slowdowns with non-fixed timezones. #30534

Mitigate date histogram slowdowns with non-fixed timezones. #30534

jpountz commented May 11, 2018

elasticmachine commented May 11, 2018

colings86 May 11, 2018

jpountz May 11, 2018

colings86 left a comment

colings86 May 11, 2018

jpountz May 11, 2018

colings86 May 11, 2018

colings86 left a comment

jpountz commented May 14, 2018

colings86 left a comment

Mitigate date histogram slowdowns with non-fixed timezones. #30534

Mitigate date histogram slowdowns with non-fixed timezones. #30534

Conversation

jpountz commented May 11, 2018

elasticmachine commented May 11, 2018

colings86 May 11, 2018

Choose a reason for hiding this comment

jpountz May 11, 2018

Choose a reason for hiding this comment

colings86 left a comment

Choose a reason for hiding this comment

colings86 May 11, 2018

Choose a reason for hiding this comment

jpountz May 11, 2018

Choose a reason for hiding this comment

colings86 May 11, 2018

Choose a reason for hiding this comment

colings86 left a comment

Choose a reason for hiding this comment

jpountz commented May 14, 2018

colings86 left a comment

Choose a reason for hiding this comment