Add notion of internal index settings #31286

jasontedor · 2018-06-13T01:28:28Z

We have some use cases for an index setting to only be manageable by dedicated APIs rather than be updateable via the update settings API. This commit adds the notion of an internal index setting. Such settings can be set on create index requests, they can not be changed via the update settings API, yet they can be changed by action on behalf of or triggered by the user via dedicated APIs.

Relates #29823

We have some use cases for an index setting to only be manageable by dedicated APIs rather than be updateable via the update settings API. This commit adds the notion of an internal index setting. Such settings can be set on create index requests, they can not be changed via the update settings API, yet they can be changed by action on behalf of or triggered by the user via dedicated APIs.

elasticmachine · 2018-06-13T01:28:29Z

Pinging @elastic/es-core-infra

jasontedor · 2018-06-13T01:35:17Z

I am open to changing the naming here. For example, another option that I considered was a "managed index setting".

jpountz · 2018-06-13T06:50:23Z

Good question. Maybe managed is better so that it is not conflated with other internal settings we have like index.version.created?

colings86

I left a comment on variable name but otherwise it looks good. I'm on the fence between Internal and Managed. On the one hand Internal seems confusing because these settings are not internal to ES itself, but Managed doesn't really convey how its going to affect the behaviour. I think I lean a bit towards keeping it as Internal and making settings like index.version.created named ReadOnly or SetOnce when we formalise those kinds of settings into a property?

colings86 · 2018-06-13T08:20:26Z

server/src/main/java/org/elasticsearch/common/settings/AbstractScopedSettings.java

+     * @param validateInternalIndex true if internal index settings should be validated
+     * @see Setting#getSettingsDependencies(String)
+     */
+    public final void validate(final Settings settings, final boolean validateDependencies, final boolean validateInternalIndex) {


Think it might be better to either make validateInternalIndex more explicit and be validateInternalIndexSettings or remove "Index" and have it as validateInternal or validateInternalSettings. The reason is that at first glance this looks like its a flag for whether we validate settings in "internal indexes".

Do you think that this isn't clear enough from the provided Javadocs? validateInternalIndexSettings is such a long name and it would cause some lines to spill over the line length limit. I am happy to adjust if you feel strongly, but I want to double check before making the change because of the reformatting it will require.

I think its ok when you read the JavaDoc but it just seems a bit strange when first reading the variable name. I agree that validateInternalIndexSettings is too long. Are you against making it validateInternalSettings instead?

I am hesitant on validateInternalSettings because we do not have a general notion of internal settings, only internal index settings for now.

Ok, thats true. In which case I'm ok with leaving this as is.

jasontedor · 2018-06-13T11:34:39Z

Good question. Maybe managed is better so that it is not conflated with other internal settings we have like index.version.created?

For today we call them "private" settings but we have not formalized them in the codebase yet. We have also considered "hidden" or "internal" for these settings; "hidden" does feel wrong to me (since they are visible in APIs) and "internal" seems more right then? So maybe "managed" is better for our use-case here indeed.

jasontedor · 2018-06-13T11:36:44Z

@jpountz Oh, I missed the comment from @colings86 about the naming in his review. I do agree that ReadOnly would better characterize what we today call private settings (like index.version.created). If we went with ReadOnly for those settings, would you be okay with InternalIndex for the settings germane to this change?

jpountz · 2018-06-13T11:39:22Z

Yes.

jasontedor · 2018-06-13T11:40:50Z

Thanks @jpountz.

colings86

LGTM

jasontedor · 2018-06-13T14:13:48Z

Thanks @colings86 and @jpountz for two productive naming discussions. Are some stars aligned? 😛

We have some use cases for an index setting to only be manageable by dedicated APIs rather than be updateable via the update settings API. This commit adds the notion of an internal index setting. Such settings can be set on create index requests, they can not be changed via the update settings API, yet they can be changed by action on behalf of or triggered by the user via dedicated APIs.

* elastic/master: (40 commits) [DOC] Extend SQL docs Immediately flush channel after writing to buffer (elastic#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (elastic#31249) move security ingest processors to a sub ingest directory (elastic#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (elastic#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (elastic#31299) CCS: don't proxy requests for already connected node (elastic#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (elastic#31298) [Rollup] Metric config parser must use builder so validation runs (elastic#31159) [ML] Check licence when datafeeds use cross cluster search (elastic#31247) Add notion of internal index settings (elastic#31286) Test: Remove broken yml test feature (elastic#31255) REST hl client: cluster health to default to cluster level (elastic#31268) [ML] Update test thresholds to account for changes to memory control (elastic#31289) ...

* elastic/master: (29 commits) [DOC] Extend SQL docs Immediately flush channel after writing to buffer (elastic#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (elastic#31249) move security ingest processors to a sub ingest directory (elastic#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (elastic#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (elastic#31299) CCS: don't proxy requests for already connected node (elastic#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (elastic#31298) [Rollup] Metric config parser must use builder so validation runs (elastic#31159) [ML] Check licence when datafeeds use cross cluster search (elastic#31247) Add notion of internal index settings (elastic#31286) Test: Remove broken yml test feature (elastic#31255) REST hl client: cluster health to default to cluster level (elastic#31268) [ML] Update test thresholds to account for changes to memory control (elastic#31289) ...

* master: Remove RestGetAllAliasesAction (#31308) Temporary fix for broken build Reenable Checkstyle's unused import rule (#31270) Remove remaining unused imports before merging #31270 Fix non-REST doc snippet [DOC] Extend SQL docs Immediately flush channel after writing to buffer (#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) CCS: don't proxy requests for already connected node (#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (#31298) [Rollup] Metric config parser must use builder so validation runs (#31159) [ML] Check licence when datafeeds use cross cluster search (#31247) Add notion of internal index settings (#31286) Test: Remove broken yml test feature (#31255) REST hl client: cluster health to default to cluster level (#31268) [ML] Update test thresholds to account for changes to memory control (#31289) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) Ignore numeric shard count if waiting for ALL (#31265) [ML] Implement new rules design (#31110) index_prefixes back-compat should test 6.3 (#30951) Core: Remove plain execute method on TransportAction (#30998) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Modify pipelining handlers to require full requests (#31280) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) Fix Netty 4 Server Transport tests. Again. REST hl client: adjust wait_for_active_shards param in cluster health (#31266) REST high-level Client: remove deprecated API methods (#31200) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) Delete typos in SAML docs (#31199) REST high-level client: add Cluster Health API (#29331) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) Remove some line length supressions (#31209) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) LLClient: Support host selection (#30523) Upgrade to Netty 4.1.25.Final (#31232) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) HLRest: Add get index templates API (#31161) Remove all unused imports and fix CRLF (#31207) [Tests] Fix self-referencing tests [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Limit the number of concurrent requests per node (#31206) Call ensureNoSelfReferences() on _agg state variable after scripted metric agg script executions (#31044) Move java version checker back to its own jar (#30708) [test] add fix for rare virtualbox error (#31212)

* 6.x: SQL: Fix build on Java 10 [Tests] Mutualize fixtures code in BaseHttpFixture (#31210) [TEST] Fix RemoteClusterClientTests#testEnsureWeReconnect [ML] Update test thresholds to account for changes to memory control (#31289) Reenable Checkstyle's unused import rule (#31270) [ML] Check licence when datafeeds use cross cluster search (#31247) Fix non-REST doc snippet [DOC] Extend SQL docs [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) SQL: Whitelist SQL utility class for better scripting (#30681) Add 5.6.11 version constant. Fix version detection. [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) Add missing release notes. Security: fix token bwc with pre 6.0.0-beta2 (#31254) Fix compilation error in UpdateSettingsIT (#31304) Test: Remove broken yml test feature (#31255) Add unreleased version 6.3.1 [Rollup] Metric config parser must use builder so validation runs (#31159) Removes experimental tag from scripted_metric aggregation (#31298) [DOCS] Removes coming tag from 6.3.0 release notes 6.3 release notes. Add notion of internal index settings (#31286) REST high-level client: add Cluster Health API (#29331) Remove leftover usage of deprecated client API SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) HLRest: Add get index templates API (#31161) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) [TEST] Mute RecoveryIT.testHistoryUUIDIsGenerated Ignore numeric shard count if waiting for ALL (#31265) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) [DOCS] Added 'fail_on_unsupported_field' param to MLT. Closes #28008 (#31160) Fix Netty 4 Server Transport tests. Again. [DOCS] Fixed typo. [DOCS] Added release highlights for 6.3 (#31256) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Upgrade to Netty 4.1.25.Final (#31232) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) [DOCS] Adds machine learning 6.3.0 release notes (#31217) Remove all unused imports and fix CRLF (#31207) [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Compliant SAML Response destination check (#31175) Move java version checker back to its own jar (#30708) TEST: Retry synced-flush if ongoing ops on primary (#30978) [test] add fix for rare virtualbox error (#31212)

jasontedor added >enhancement review :Core/Infra/Settings Settings infrastructure and APIs v7.0.0 v6.4.0 labels Jun 13, 2018

jasontedor requested review from colings86 and talevy June 13, 2018 01:28

elasticmachine mentioned this pull request Jun 13, 2018

[meta] Index Lifecycle Management Plan #29823

Closed

comment

ab24609

colings86 reviewed Jun 13, 2018

View reviewed changes

colings86 approved these changes Jun 13, 2018

View reviewed changes

jasontedor merged commit 7199d5f into elastic:master Jun 13, 2018

jasontedor deleted the index-internal-settings branch June 13, 2018 14:28

davidkyle mentioned this pull request Jun 13, 2018

Fix compilation error in UpdateSettingsIT #31304

Merged

jasontedor mentioned this pull request Aug 31, 2018

Introduce private settings #33327

Merged

colings86 added the v7.0.0-beta1 label Feb 7, 2019

colings86 removed the v7.0.0 label Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add notion of internal index settings #31286

Add notion of internal index settings #31286

jasontedor commented Jun 13, 2018

elasticmachine commented Jun 13, 2018

jasontedor commented Jun 13, 2018

jpountz commented Jun 13, 2018

colings86 left a comment

colings86 Jun 13, 2018

jasontedor Jun 13, 2018

colings86 Jun 13, 2018

jasontedor Jun 13, 2018

colings86 Jun 13, 2018

jasontedor commented Jun 13, 2018

jasontedor commented Jun 13, 2018

jpountz commented Jun 13, 2018

jasontedor commented Jun 13, 2018

colings86 left a comment

jasontedor commented Jun 13, 2018

Add notion of internal index settings #31286

Add notion of internal index settings #31286

Conversation

jasontedor commented Jun 13, 2018

elasticmachine commented Jun 13, 2018

jasontedor commented Jun 13, 2018

jpountz commented Jun 13, 2018

colings86 left a comment

Choose a reason for hiding this comment

colings86 Jun 13, 2018

Choose a reason for hiding this comment

jasontedor Jun 13, 2018

Choose a reason for hiding this comment

colings86 Jun 13, 2018

Choose a reason for hiding this comment

jasontedor Jun 13, 2018

Choose a reason for hiding this comment

colings86 Jun 13, 2018

Choose a reason for hiding this comment

jasontedor commented Jun 13, 2018

jasontedor commented Jun 13, 2018

jpountz commented Jun 13, 2018

jasontedor commented Jun 13, 2018

colings86 left a comment

Choose a reason for hiding this comment

jasontedor commented Jun 13, 2018