Skip to content

Conversation

@Tim-Brooks
Copy link
Contributor

Traditionally remote clusters can be configured dynamically. However,
the compress and ping settings are not currently set to be configured
dynamically. This commit changes that.

Traditionally remote clusters can be configured dynamically. However,
the compress and ping settings are not currently set to be configured
dynamically. This commit changes that.
@Tim-Brooks Tim-Brooks added >non-issue :Distributed Coordination/Network Http and internode communication implementations v7.0.0 v6.7.0 labels Jan 7, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@Tim-Brooks
Copy link
Contributor Author

I have opened #37201 to track additional work we might want done in this area.

@Tim-Brooks
Copy link
Contributor Author

@dliappis I did not get a review for this today, so I was not able to merge it. But if it looks good to you, you can merge it. If you do, just mark the backport pending. And I will backport it.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM. Let's follow-up with responding to changes in these settings on an existing remote cluster.

@Tim-Brooks Tim-Brooks merged commit b066596 into elastic:master Jan 8, 2019
@dliappis
Copy link
Contributor

dliappis commented Jan 8, 2019

Also LGTM from me, thanks @tbrooks8 ; I have tested it on master and the setting works fine and can be seen enabled in cluster settings.

@javanna
Copy link
Member

javanna commented Jan 8, 2019

heya I was looking at this with @dliappis and we don't see the setting being applied, even when specified while registering a new cluster. I understand from the description that it's expected that the setting does not currently get updated for existing clusters (as there is no consumer), but I would expect it to be applied to a newly built connection profile created when registering a new cluster. This should be reproducible with a unit test.

@Tim-Brooks
Copy link
Contributor Author

Yeah it looks like there will be more steps involved to use the dynamic settings when adding a remote cluster. I marked the related issue as a bug and it will be fixed with that.

@Tim-Brooks Tim-Brooks deleted the settings_dynamic branch December 18, 2019 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Network Http and internode communication implementations >non-issue v6.7.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants