Skip to content

Make remote ping and compress settings dynamic#37200

Merged
Tim-Brooks merged 2 commits intoelastic:masterfrom
Tim-Brooks:settings_dynamic
Jan 8, 2019
Merged

Make remote ping and compress settings dynamic#37200
Tim-Brooks merged 2 commits intoelastic:masterfrom
Tim-Brooks:settings_dynamic

Conversation

@Tim-Brooks
Copy link
Copy Markdown
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/Network Http and internode communication implementations v7.0.0 v6.7.0 labels Jan 7, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed

@Tim-Brooks
Copy link
Copy Markdown
Contributor Author

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

@Tim-Brooks
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

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
Copy Markdown
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/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