Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove timezone validation on rollup range queries #40647

Merged

Conversation

polyfractal
Copy link
Contributor

We enforced the timezone of range queries when using the rollup search endpoint, but this validation is not needed. Since rollup dates are stored in UTC, and range queries are always converted to UTC (even if specifying a time_zone) the validation is not needed and can prevent legitimate queries from running.

We enforced the timezone of range queries when using the rollup
search endpoint, but this validation is not needed.  Since
rollup dates are stored in UTC, and range queries are always
converted to UTC (even if specifying a `time_zone`) the validation
is not needed and can prevent legitimate queries from running.
@polyfractal polyfractal added >bug v7.0.0 :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data v8.0.0 v7.2.0 v6.7.1 labels Mar 29, 2019
@polyfractal polyfractal requested a review from jimczi March 29, 2019 18:32
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@colings86 colings86 added v6.7.2 and removed v6.7.1 labels Mar 30, 2019
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@polyfractal
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

@polyfractal
Copy link
Contributor Author

@elasticmachine test this please

@rayafratkina
Copy link

rayafratkina commented Apr 2, 2019

Note that we have a blocker on Kibana caused by this: elastic/kibana#33974
We don't have a workaround so from our pov, this issue needs to be a blocker or rollups will remain broken in the UI.

@polyfractal
Copy link
Contributor Author

@rayafratkina When I discussed with @jasontedor earlier we decided it didn't qualify as a blocker for the 7.0 train because Rollup is still an experimental feature, and this only impacts dashboards that are using non-UTC timezones.

Also, this validation has been there since 6.4 so I'm surprised dashboards are/were working at all with non-UTC timezones (from the timepicker range, not the date_histo config).

@polyfractal polyfractal merged commit 6667bd8 into elastic:master Apr 2, 2019
polyfractal added a commit that referenced this pull request Apr 2, 2019
We enforced the timezone of range queries when using the rollup
search endpoint, but this validation is not needed.  Since
rollup dates are stored in UTC, and range queries are always
converted to UTC (even if specifying a `time_zone`) the validation
is not needed and can prevent legitimate queries from running.
polyfractal added a commit that referenced this pull request Apr 2, 2019
We enforced the timezone of range queries when using the rollup
search endpoint, but this validation is not needed.  Since
rollup dates are stored in UTC, and range queries are always
converted to UTC (even if specifying a `time_zone`) the validation
is not needed and can prevent legitimate queries from running.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 2, 2019
* master:
  add reason to DataFrameTransformState and add hlrc protocol tests (elastic#40736)
  Remove timezone validation on rollup range queries (elastic#40647)
  Fix testRunStateChangePolicyWithAsyncActionNextStep race condition (elastic#40707)
  Don't mark shard as refreshPending on stats fetching (elastic#40458)
  Name Snapshot Data Blobs by UUID (elastic#40652)
  SQL: [TEST] Mute TIME related failing tests
  [TEST] RecoveryWithConcurrentIndexing test (elastic#40733)
@colings86 colings86 added v6.7.1 and removed v6.7.2 labels Apr 3, 2019
@colings86
Copy link
Contributor

@polyfractal this looks like its still pending the backport for 6.7, is that correct? I'm going to label it assuming this is correct for now

@polyfractal
Copy link
Contributor Author

Sorry yes, still waiting on backport to 6.7

polyfractal added a commit that referenced this pull request Apr 5, 2019
We enforced the timezone of range queries when using the rollup
search endpoint, but this validation is not needed.  Since
rollup dates are stored in UTC, and range queries are always
converted to UTC (even if specifying a `time_zone`) the validation
is not needed and can prevent legitimate queries from running.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
We enforced the timezone of range queries when using the rollup
search endpoint, but this validation is not needed.  Since
rollup dates are stored in UTC, and range queries are always
converted to UTC (even if specifying a `time_zone`) the validation
is not needed and can prevent legitimate queries from running.
@jeremygachet
Copy link

Hello,
we still have the same bug in kibana.
This fix has been merged ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data v6.7.2 v7.0.0-rc2 v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants