Dynamically enable / disable plugins in correspondence to stateless mode.#142147
Dynamically enable / disable plugins in correspondence to stateless mode.#142147mosche merged 24 commits intoelastic:mainfrom
Conversation
…ode. The PluginDescriptor property `distribution.mode` can be optionally set to one of the following: - STATEFUL_ONLY (exclude plugin if stateless mode is enabled) - STATELESS_ONLY (exclude plugin if stateless mode is disabled) - ALWAYS (the default to always include the plugin) This provides a way to dynamically refine the current distribution and exclude plugins that may not be present in stateful or stateless respectively. Relates to ES-13956
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
kingherc
left a comment
There was a problem hiding this comment.
LGTM, thanks Moritz! Feel free to see whether waiting for a Core/Infra approval as well would be good.
kingherc
left a comment
There was a problem hiding this comment.
Based on discussion, some more comments. Also can you make x-pack-monitoring STATEFUL_ONLY?
rjernst
left a comment
There was a problem hiding this comment.
Looks pretty good, a few thoughts
| private static Optional<DeploymentTarget> readDeploymentTarget(Map<String, String> propsMap, String pluginId) { | ||
| String rawValue = propsMap.remove("deployment.target"); | ||
| if (rawValue == null || rawValue.isBlank()) { | ||
| return Optional.empty(); |
There was a problem hiding this comment.
Do we need both Optional here and the default of ALWAYS? Could we just use ALWAYS and remove Optional?
Also, nit: instead of allowing "blank" to be the default could we error, ie enforce if the property is present, it must be one of the 3 values?
There was a problem hiding this comment.
Just thinking, if we enforce this, would it maybe break 3rd party custom plugins?
There was a problem hiding this comment.
I wasn't suggesting we require the property. Rather, when the property isn't set, the default of ALWAYS is used, rather than Optional.empty() which later gets interpreted to ALWAYS.
There was a problem hiding this comment.
@rjernst I can do that. Though, one reason I decided for Optional was to not having to fallback to ALWAYS when reading a plugin descriptor from StreamInput (rather than from the descriptor file). Since this is really only relevant prior to loading the plugin, I decided against even serializing it (and having to account for different transport versions) as it's rather pointless at that point. If it wasn't supposed to be loaded, it won't be loaded - and won't be present.
There was a problem hiding this comment.
@rjernst Finally picking this up again. Any thought on above?
There was a problem hiding this comment.
I removed all optionals, BUT still not serializing deploymentTarget (neither binary, nor XContent).
When reading from StreamInput the target is always ALL. But at that point's it's not relevant anymore.
server/src/main/java/org/elasticsearch/plugins/PluginDescriptor.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/plugins/PluginDescriptor.java
Outdated
Show resolved
Hide resolved
masseyke
left a comment
There was a problem hiding this comment.
I think my concerns are already addressed in other comments. Otherwise, LGTM
💔 Backport failed
You can use sqren/backport to manually backport by running |
…ode. (elastic#142147) deploymentTarget provides a way to dynamically refine which plugins are loaded for a deployment based on stateless.enabled: - STATEFUL_ONLY: Only load plugin on stateful deployments (stateless mode disabled) - STATELESS_ONLY: Only load plugin on stateless deployments (stateless mode enabled) - ALL: Always load plugin (default) Relates to ES-13956 Relates to ES-13956
…locations * upstream/main: (126 commits) Update KnnIndexTester to use more settings from datasets (elastic#143869) fix: dynamic template vector array is overridden by automatic dense_vector mapping (elastic#143733) ES|QL: Don't reuse the same alias for _fork column (elastic#143909) Close and initialize clients after each node upgrade in logsdb rolling upgrade tests. (elastic#143823) ESQL: Added GroupedTopNOperator for LIMIT BY, compute only (elastic#143476) Handle views in ResolveIndexAction (elastic#143561) Improve reindex rethrottle API in stateless (elastic#143771) Use a copy of the SearchExecutionContext for each Percolator execution (elastic#142765) Log the stacktrace when we encounter a deprecation warning for `default_metric` (elastic#143929) ESQL: evaluate ReferenceAttributes to potentially FieldAttributes for full-text functions restriction (elastic#143893) Add ClusterStateSerializationStats Serializatation Tests (elastic#142703) Adds Coordination Diagnostics Tests (elastic#142709) Upgrade Elasticsearch to Apache Lucene 10.4 (elastic#141882) ESQL: Add configurable bracket-based multi-value support for CSV reader (elastic#143890) time series es819 binary dv use up to a 1mb block size (elastic#143049) Dynamically enable / disable plugins in correspondence to stateless mode. (elastic#142147) ES|QL: Implement first/last_over_time for tdigest (elastic#143832) Document CHANGE_POINT limitation (elastic#143877) Fix OperationsOnSeqNoDisabledIndicesIT (elastic#143892) [Test] Test that sequence numbers are not pruned with retention lease (elastic#143825) ...
deploymentTargetprovides a way to dynamically refine which plugins are loaded in a deployment based onstateless.enabled:STATEFUL_ONLY: Only load plugin on stateful deployments (stateless mode disabled)STATELESS_ONLY: Only load plugin on stateless deployments (stateless mode enabled)ALL: Always load plugin (default)Relates to ES-13956