Skip to content

Conversation

@Tim-Brooks
Copy link
Contributor

In order to use the "proxy" name for the new connection mode, the
existing cluster.remote.cluster_name.proxy setting must be renamed
to cluster.remote.cluster_name.sniff.proxy. This commit implements
this change and adds a setting upgrader. Since the setting is not
documented, the old setting is immediately removed. Finally, the
settings upgrader infrastructure is modified so that settings that are
not registered with the cluster can be upgraded.

@Tim-Brooks Tim-Brooks added >non-issue :Distributed Coordination/Network Http and internode communication implementations v8.0.0 v7.6.0 labels Dec 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Network)

@Tim-Brooks
Copy link
Contributor Author

This commit is necessary for #50291. Otherwise the news settings cannot be added.

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

There are some relevant REST tests failing. I also wonder how the upgrade path for a rolling upgrade will look like.

final ClusterGetSettingsResponse clusterGetSettingsResponse = ClusterGetSettingsResponse.fromXContent(parser);
final Settings settings = clusterGetSettingsResponse.getPersistentSettings();

logger.error(settings);
Copy link
Contributor

Choose a reason for hiding this comment

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

?

@Tim-Brooks
Copy link
Contributor Author

Tim-Brooks commented Dec 18, 2019

After looking into this, I am not sure if using the name proxy is going to work. As I investigated the rolling restart tests I had two concerns:

  1. It seems like there might be races between when our settings upgraders run. For example a node might upgrade its cluster state on a restart, but that might be overwritten when it receives the cluster state from another node.
  2. A concern for this specific issue, newly upgraded non-master nodes will receive a cluster state with a setting that does not exist anymore. It is tricky to figure out the implications of this, but I was seeing the setting changed to be prefixed by "archived." which is the marker of an invalid setting.

I am open to any discussions about how to resolve this issue. If there is some safe way to ensure the handling of a removed setting I can go that direction. But we can also consider other names such as cluster.remote.cluster_name.proxy_mode.address. Maybe in that case we would want to name to also be cluster.remote.cluster_name.sniff_mode.seeds.

@Tim-Brooks Tim-Brooks requested a review from ywelsch December 18, 2019 18:56
@Tim-Brooks
Copy link
Contributor Author

This PR will probably be closed in favor of #50291.

@Tim-Brooks
Copy link
Contributor Author

Closing. We are not moving in this direction.

@Tim-Brooks Tim-Brooks closed this Dec 19, 2019
@Tim-Brooks Tim-Brooks deleted the remote_cluster_rename_proxy_setting branch April 30, 2020 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants