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

Add a deprecation notice to shadow replicas #22647

Merged
merged 18 commits into from Jan 18, 2017

Conversation

Projects
None yet
4 participants
@bleskes
Copy link
Member

bleskes commented Jan 16, 2017

Relates to #22024

On top of documentation, the PR adds deprecation loggers and deals with the resulting warning headers.

The yaml test is set exclude versions up to 6.0. This is need to make sure bwc tests pass until this is backported to 5.2.0 . Once that's done, I will change the yaml test version limits

@dakrone
Copy link
Member

dakrone left a comment

LGTM

@jasontedor
Copy link
Member

jasontedor left a comment

I left some nits.

@@ -1217,6 +1218,7 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti
* {@link #isIndexUsingShadowReplicas(org.elasticsearch.common.settings.Settings)}.
*/
public static boolean isOnSharedFilesystem(Settings settings) {
// don't use the settings directly, not to trigger manny deprecation logging

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

Nits: settings -> setting and not to trigger manny -> to not trigger verbose (it doesn't have to be exactly this, but at least fix manny)

@@ -1226,6 +1228,7 @@ public static boolean isOnSharedFilesystem(Settings settings) {
* setting for this is <code>false</code>.
*/
public static boolean isIndexUsingShadowReplicas(Settings settings) {
// don't use the settings directly, not to trigger manny deprecation logging

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

Nits: settings -> setting and not to trigger manny -> to not trigger verbose (it doesn't have to be exactly this, but at least fix manny)

@@ -476,8 +476,10 @@ static NodeShardsResult buildVersionBasedNodeShardsResult(ShardRouting shard, bo
* recovered on any node
*/
private boolean recoverOnAnyNode(IndexMetaData metaData) {
// don't use the settings directly, not to trigger manny deprecation logging

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

Nits: settings -> setting and not to trigger manny -> to not trigger verbose (it doesn't have to be exactly this, but at least fix manny)

*/
@Override
protected boolean enableWarningsCheck() {
return false;

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

Would you please add an assert guarding against this staying around after shadow replicas are removed?

This comment has been minimized.

Copy link
@bleskes

bleskes Jan 17, 2017

Author Member

I did :) that's why the java docs links to the setting in question

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

Okay, I'm fine with that.

*/
@Override
protected boolean enableWarningsCheck() {
return false;

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

Would you please add an assert guarding against this staying around after shadow replicas are removed?

This comment has been minimized.

Copy link
@bleskes

bleskes Jan 17, 2017

Author Member

same as above

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

Yes, I'm also fine with it here.

@@ -1,7 +1,7 @@
[[indices-shadow-replicas]]
== Shadow replica indices

experimental[]
deprecated[5.2.0, Shadow replicas don't see much usage and we are planning to remove them]

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

"Planning" or "going"?

This comment has been minimized.

Copy link
@bleskes

bleskes Jan 17, 2017

Author Member

I don't know. Chose for a softer language here.


=== Shadow Replicas are deprecated

<<indices-shadow-replicas,Shadow Replicas>> don't see much usage and we are planning to remove them.

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jan 17, 2017

Member

"Planning" or "going"?

@jasontedor

This comment has been minimized.

Copy link
Member

jasontedor commented Jan 17, 2017

Thanks @bleskes. Will you open a follow-up issue for path.shared_data?

@clintongormley clintongormley added >deprecation and removed >docs labels Jan 17, 2017

bleskes added some commits Jan 17, 2017

@bleskes

This comment has been minimized.

Copy link
Member Author

bleskes commented Jan 17, 2017

@jasontedor I pushed another commit. Can you look?

@jasontedor
Copy link
Member

jasontedor left a comment

LGTM.

@bleskes

This comment has been minimized.

Copy link
Member Author

bleskes commented Jan 18, 2017

retest this please

@bleskes

This comment has been minimized.

Copy link
Member Author

bleskes commented Jan 18, 2017

Retest this please

@bleskes

This comment has been minimized.

Copy link
Member Author

bleskes commented Jan 18, 2017

retest this please

@bleskes bleskes merged commit 1227044 into elastic:master Jan 18, 2017

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details

@bleskes bleskes deleted the bleskes:deprecate_shadow_replicas branch Jan 18, 2017

s1monw added a commit to s1monw/elasticsearch that referenced this pull request Jan 18, 2017

Streamline foreign stored context restore and allow to perseve respon…
…se headers

Today we do not preserve response headers if they are present on a transport protocol
response. While preserving these headers is not always desired, in the most cases we
should pass on these headers to have consistent results for deprectatoin headers etc.
yet, this hasn't been much of a problem since most of the deprecations are detected early
ie. on the coordinating node such that this bug wasn't uncovered until elastic#22647

This commit allow to optinally preserve headers when a context is restored and also streamlines
the context restore since it leaked frequently into the callers thread context when the callers
context wasn't restored again.

bleskes added a commit that referenced this pull request Jan 18, 2017

Add a deprecation notice to shadow replicas (#22647)
Relates to #22024

On top of documentation, the PR adds deprecation loggers and deals with the resulting warning headers.

The yaml test is set exclude versions up to 6.0. This is need to make sure bwc tests pass until this is backported to 5.2.0 . Once that's done, I will change the yaml test version limits

s1monw added a commit that referenced this pull request Jan 18, 2017

Streamline foreign stored context restore and allow to perserve respo…
…nse headers (#22677)

Today we do not preserve response headers if they are present on a transport protocol
response. While preserving these headers is not always desired, in the most cases we
should pass on these headers to have consistent results for depreciation headers etc.
yet, this hasn't been much of a problem since most of the deprecations are detected early
ie. on the coordinating node such that this bug wasn't uncovered until #22647

This commit allow to optionally preserve headers when a context is restored and also streamlines
the context restore since it leaked frequently into the callers thread context when the callers
context wasn't restored again.

s1monw added a commit that referenced this pull request Jan 18, 2017

Streamline foreign stored context restore and allow to perserve respo…
…nse headers (#22677)

Today we do not preserve response headers if they are present on a transport protocol
response. While preserving these headers is not always desired, in the most cases we
should pass on these headers to have consistent results for depreciation headers etc.
yet, this hasn't been much of a problem since most of the deprecations are detected early
ie. on the coordinating node such that this bug wasn't uncovered until #22647

This commit allow to optionally preserve headers when a context is restored and also streamlines
the context restore since it leaked frequently into the callers thread context when the callers
context wasn't restored again.

s1monw added a commit that referenced this pull request Jan 19, 2017

Streamline foreign stored context restore and allow to perserve respo…
…nse headers (#22677)

Today we do not preserve response headers if they are present on a transport protocol
response. While preserving these headers is not always desired, in the most cases we
should pass on these headers to have consistent results for depreciation headers etc.
yet, this hasn't been much of a problem since most of the deprecations are detected early
ie. on the coordinating node such that this bug wasn't uncovered until #22647

This commit allow to optionally preserve headers when a context is restored and also streamlines
the context restore since it leaked frequently into the callers thread context when the callers
context wasn't restored again.

bleskes added a commit that referenced this pull request Jan 19, 2017

Add a deprecation notice to shadow replicas (#22647)
Relates to #22024

On top of documentation, the PR adds deprecation loggers and deals with the resulting warning headers.

The yaml test is set exclude versions up to 6.0. This is need to make sure bwc tests pass until this is backported to 5.2.0 . Once that's done, I will change the yaml test version limits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.