Skip to content

Allow query caching by default again#33328

Merged
jasontedor merged 3 commits intoelastic:masterfrom
jasontedor:opt-out-query-cache-default-distribution
Sep 4, 2018
Merged

Allow query caching by default again#33328
jasontedor merged 3 commits intoelastic:masterfrom
jasontedor:opt-out-query-cache-default-distribution

Conversation

@jasontedor
Copy link
Copy Markdown
Member

With the introduction of the default distribution, it means that by default the query cache is wrapped in the security implementation of the query cache. This cache does not allow caching if the request does not carry indices permissions. Yet, this will not happen if authorization is not allowed, which it is not by default. This means that with the introduction of the default distribution, query caching was disabled by default! This commit addresses this by checking if authorization is allowed and if not, delegating to the default indices query cache. Otherwise, we proceed as before with security. Additionally, we clear the cache on license state changes.

Closes #33191

With the introduction of the default distribution, it means that by
default the query cache is wrapped in the security implementation of the
query cache. This cache does not allow caching if the request does not
carry indices permissions. Yet, this will not happen if authorization is
not allowed, which it is not by default. This means that with the
introduction of the default distribution, query caching was disabled by
default! This commit addresses this by checking if authorization is
allowed and if not, delegating to the default indices query
cache. Otherwise, we proceed as before with security. Additionally, we
clear the cache on license state changes.
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security

Copy link
Copy Markdown
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

I left a question,

…e-default-distribution

* elastic/master: (213 commits)
  ML: Fix build after HLRC change
  Fix inner hits retrieval when stored fields are disabled (_none_) (elastic#33018)
  SQL: Show/desc commands now support table ids (elastic#33363)
  Mute testValidateFollowingIndexSettings
  HLRC: Add delete by query API (elastic#32782)
  [ML] The sort field on get records should default to the record_score (elastic#33358)
  [ML] Minor improvements to categorization Grok pattern creation (elastic#33353)
  [DOCS] fix a couple of typos (elastic#33356)
  Disable assemble task instead of removing it (elastic#33348)
  Simplify the return type of FieldMapper#parse. (elastic#32654)
  [ML] Delete forecast API (elastic#31134) (elastic#33218)
  Introduce private settings (elastic#33327)
  [Docs] Add search timeout caveats (elastic#33354)
  TESTS: Fix Race Condition in Temp Path Creation (elastic#33352)
  Fix from_range in search_after in changes snapshot (elastic#33335)
  TESTS+DISTR.: Fix testIndexCheckOnStartup Flake (elastic#33349)
  Null completion field should not throw IAE (elastic#33268)
  Adds code to help with IndicesRequestCacheIT failures (elastic#33313)
  Prevent NPE parsing the stop datafeed request. (elastic#33347)
  HLRC: Add ML get overall buckets API (elastic#33297)
  ...
Copy link
Copy Markdown
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM

@jasontedor jasontedor merged commit 48790b0 into elastic:master Sep 4, 2018
jasontedor added a commit that referenced this pull request Sep 4, 2018
With the introduction of the default distribution, it means that by
default the query cache is wrapped in the security implementation of the
query cache. This cache does not allow caching if the request does not
carry indices permissions. Yet, this will not happen if authorization is
not allowed, which it is not by default. This means that with the
introduction of the default distribution, query caching was disabled by
default! This commit addresses this by checking if authorization is
allowed and if not, delegating to the default indices query
cache. Otherwise, we proceed as before with security. Additionally, we
clear the cache on license state changes.
jasontedor added a commit that referenced this pull request Sep 4, 2018
With the introduction of the default distribution, it means that by
default the query cache is wrapped in the security implementation of the
query cache. This cache does not allow caching if the request does not
carry indices permissions. Yet, this will not happen if authorization is
not allowed, which it is not by default. This means that with the
introduction of the default distribution, query caching was disabled by
default! This commit addresses this by checking if authorization is
allowed and if not, delegating to the default indices query
cache. Otherwise, we proceed as before with security. Additionally, we
clear the cache on license state changes.
@jasontedor jasontedor deleted the opt-out-query-cache-default-distribution branch September 4, 2018 21:57
@leeeboo
Copy link
Copy Markdown

leeeboo commented Sep 5, 2018

@jasontedor Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants