Add a deprecation notice to shadow replicas #22025
Conversation
I felt we can't mark deprecated in hind sight... on the other hand it does make it for people. Doubting. Do others have opinions? |
|
don't we usually include in the deprecation note the first version that had the feature deprecated? I think it can't be 5.0.0 although it would be technically possible as there are no code changes involved. |
There is a word missing here so I'm not sure what you're saying (sorry), but my thinking is that people might land on the 5.0 docs from a search engine or elsewhere and not necessarily think to click to 5.1 or current. I'd hate for someone impacted by this to miss the notice. |
It's me who should be saying sorry (the missing word is visible). I was torn between your point of view (let as many people as possible know) and the one of Luca. @clintongormley it looks like you removed some labels - do you have an opinion here? |
|
Maybe we should also log deprecation warnings when the feature is used? (not sure how feasible that is, and should not flood the logs, but at least we would make sure that people notice without having to look at the docs) |
|
@bleskes re labels, just preparing the 5.1.1. release notes I'm OK with deprecating it now (as in whichever version it lands in) but let's make sure there is deprecation logging |
|
@clintongormley I added deprecation logging and updated the messages a bit. It now looks like this (when you create a shadow replicas index):
@jasontedor can you take another look? |
|
Thx @jasontedor |
…ly as it triggers (many) deprecation logging #22025 deprecated this setting (pending it's removal) but it's frequent usage will spam the deprecation logs and also fails test. As temporary work around we should not use the setting object directly.
|
I reverted this PR as it triggers warnings about usage of deprecated features. My bad for not catching it before merging |
* master: (131 commits) Replace EngineClosedException with AlreadyClosedExcpetion (elastic#22631) Remove HttpServer and HttpServerAdapter in favor of a simple dispatch method (elastic#22636) move ignore parameter support from yaml test client to low level rest client (elastic#22637) Fix thread safety of Stempel's token filter factory (elastic#22610) Update profile.asciidoc Wrap rest httpclient with doPrivileged blocks (elastic#22603) Revert "Add a deprecation notice to shadow replicas (elastic#22025)" Revert "Don'y use `INDEX_SHARED_FS_ALLOW_RECOVERY_ON_ANY_NODE_SETTING` directly as it triggers (many) deprecation logging" Don'y use `INDEX_SHARED_FS_ALLOW_RECOVERY_ON_ANY_NODE_SETTING` directly as it triggers (many) deprecation logging Add a deprecation notice to shadow replicas (elastic#22025) [Docs] Fix section title in profile.asciidoc ProfileResult and CollectorResult should print machine readable timing information (elastic#22561) Add replica ops with version conflict to translog Replace custom Functional interface in ElasticsearchException with CheckedFunction Make RestChannelConsumer extend CheckedConsumer<RestChannel, Exception> replace ShardSearchRequest.FilterParser functional interface with CheckedFunction replace custom functional interface with CheckedFunction in percolate module [TEST] replace SizeFunction with Function<Integer, Integer> Expose CheckedFunction Expose logs base path ...
|
This PR deprecates shadow replicas settings and the custom data path support. Upon discussion we have decided not to deprecate the custom data path settings. In order to avoid confusion I opened a new PR - #22647 - and remove the labels from this one. |
See #22024