Ensure default watches are updated for rolling upgrades. #57185
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
then shutdown old instances), the watches that ship by default with monitoring may not get properly updated to the new version. Monitoring watches can only get published if the internal state is marked as dirty. If a node is not master, will also get marked as clean (e.g. not dirty). For a mixed cluster upgrade, it is possible for the new node to be added, not as master, the internal state gets marked as clean so that no more attempts can be made to publish the watches. This happens on all new nodes. Once the old nodes are de-commissioned one of the new version nodes in the cluster gets promoted to master. However, that new master node (with out intervention like restarting the node or removing/adding exporters) will never attempt to re-publish since the internal state was already marked as clean. This commit adds a cluster state listener to mark the resource dirty when a node is promoted to master. This will allow the new resource to be published without any intervention.
|
Pinging @elastic/es-core-features (:Core/Features/Watcher) |
elasticmachine
added
the
Team:Data Management
danhermann
reviewed
Jun 1, 2020
Comment on lines
+430
to
+434
| onLocalMasterListener = clusterChangedEvent -> { | ||
| if (clusterChangedEvent.nodesDelta().masterNodeChanged() && clusterChangedEvent.localNodeMaster()) { | ||
| resource.markDirty(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The way I read this, the resource will be marked as dirty any time the node becomes master, not just when it has become master after a mixed cluster upgrade. Am I reading that wrong or is that acceptable?
There was a problem hiding this comment.
the resource will be marked as dirty any time the node becomes master
That is correct. However, I am not concerned about the overhead since marking it dirty it will only check to see if the resource exists (and the license level is OK). It will not re-put the watch... even it if it did, it would be functional no-op (putting the same thing that already exists). This all happens on the monitoring thread too, so no worry about extra work (except setting a boolean) on the cluster state applier thread.
|
thanks for the review @danhermann |
jakelandis
added a commit
to jakelandis/elasticsearch
that referenced
this pull request
Jun 2, 2020
For a rolling/mixed cluster upgrade (add new version to existing cluster then shutdown old instances), the watches that ship by default with monitoring may not get properly updated to the new version. Monitoring watches can only get published if the internal state is marked as dirty. If a node is not master, will also get marked as clean (e.g. not dirty). For a mixed cluster upgrade, it is possible for the new node to be added, not as master, the internal state gets marked as clean so that no more attempts can be made to publish the watches. This happens on all new nodes. Once the old nodes are de-commissioned one of the new version nodes in the cluster gets promoted to master. However, that new master node (with out intervention like restarting the node or removing/adding exporters) will never attempt to re-publish since the internal state was already marked as clean. This commit adds a cluster state listener to mark the resource dirty when a node is promoted to master. This will allow the new resource to be published without any intervention.
jakelandis
added a commit
to jakelandis/elasticsearch
that referenced
this pull request
Jun 2, 2020
For a rolling/mixed cluster upgrade (add new version to existing cluster then shutdown old instances), the watches that ship by default with monitoring may not get properly updated to the new version. Monitoring watches can only get published if the internal state is marked as dirty. If a node is not master, will also get marked as clean (e.g. not dirty). For a mixed cluster upgrade, it is possible for the new node to be added, not as master, the internal state gets marked as clean so that no more attempts can be made to publish the watches. This happens on all new nodes. Once the old nodes are de-commissioned one of the new version nodes in the cluster gets promoted to master. However, that new master node (with out intervention like restarting the node or removing/adding exporters) will never attempt to re-publish since the internal state was already marked as clean. This commit adds a cluster state listener to mark the resource dirty when a node is promoted to master. This will allow the new resource to be published without any intervention.
jakelandis
added a commit
to jakelandis/elasticsearch
that referenced
this pull request
Jun 2, 2020
For a rolling/mixed cluster upgrade (add new version to existing cluster then shutdown old instances), the watches that ship by default with monitoring may not get properly updated to the new version. Monitoring watches can only get published if the internal state is marked as dirty. If a node is not master, will also get marked as clean (e.g. not dirty). For a mixed cluster upgrade, it is possible for the new node to be added, not as master, the internal state gets marked as clean so that no more attempts can be made to publish the watches. This happens on all new nodes. Once the old nodes are de-commissioned one of the new version nodes in the cluster gets promoted to master. However, that new master node (with out intervention like restarting the node or removing/adding exporters) will never attempt to re-publish since the internal state was already marked as clean. This commit adds a cluster state listener to mark the resource dirty when a node is promoted to master. This will allow the new resource to be published without any intervention.
jakelandis
added a commit
that referenced
this pull request
Jun 4, 2020
… (#57563) For a rolling/mixed cluster upgrade (add new version to existing cluster then shutdown old instances), the watches that ship by default with monitoring may not get properly updated to the new version. Monitoring watches can only get published if the internal state is marked as dirty. If a node is not master, will also get marked as clean (e.g. not dirty). For a mixed cluster upgrade, it is possible for the new node to be added, not as master, the internal state gets marked as clean so that no more attempts can be made to publish the watches. This happens on all new nodes. Once the old nodes are de-commissioned one of the new version nodes in the cluster gets promoted to master. However, that new master node (with out intervention like restarting the node or removing/adding exporters) will never attempt to re-publish since the internal state was already marked as clean. This commit adds a cluster state listener to mark the resource dirty when a node is promoted to master. This will allow the new resource to be published without any intervention.
jakelandis
added a commit
that referenced
this pull request
Jun 4, 2020
… (#57562) For a rolling/mixed cluster upgrade (add new version to existing cluster then shutdown old instances), the watches that ship by default with monitoring may not get properly updated to the new version. Monitoring watches can only get published if the internal state is marked as dirty. If a node is not master, will also get marked as clean (e.g. not dirty). For a mixed cluster upgrade, it is possible for the new node to be added, not as master, the internal state gets marked as clean so that no more attempts can be made to publish the watches. This happens on all new nodes. Once the old nodes are de-commissioned one of the new version nodes in the cluster gets promoted to master. However, that new master node (with out intervention like restarting the node or removing/adding exporters) will never attempt to re-publish since the internal state was already marked as clean. This commit adds a cluster state listener to mark the resource dirty when a node is promoted to master. This will allow the new resource to be published without any intervention.
jakelandis
added a commit
that referenced
this pull request
Jun 4, 2020
… (#57561) For a rolling/mixed cluster upgrade (add new version to existing cluster then shutdown old instances), the watches that ship by default with monitoring may not get properly updated to the new version. Monitoring watches can only get published if the internal state is marked as dirty. If a node is not master, will also get marked as clean (e.g. not dirty). For a mixed cluster upgrade, it is possible for the new node to be added, not as master, the internal state gets marked as clean so that no more attempts can be made to publish the watches. This happens on all new nodes. Once the old nodes are de-commissioned one of the new version nodes in the cluster gets promoted to master. However, that new master node (with out intervention like restarting the node or removing/adding exporters) will never attempt to re-publish since the internal state was already marked as clean. This commit adds a cluster state listener to mark the resource dirty when a node is promoted to master. This will allow the new resource to be published without any intervention.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For a rolling/mixed cluster upgrade (add new version to existing cluster
then shutdown old instances), the watches that ship by default
with monitoring may not get properly updated to the new version.
Monitoring watches can only get published if the internal state is
marked as dirty. If a node is not master, will also get marked as
clean (e.g. not dirty).
For a mixed cluster upgrade, it is possible for the new node to be
added, not as master, the internal state gets marked as clean so
that no more attempts can be made to publish the watches. This
happens on all new nodes. Once the old nodes are de-commissioned
one of the new version nodes in the cluster gets promoted to master.
However, that new master node (with out intervention like restarting
the node or removing/adding exporters) will never attempt to re-publish
since the internal state was already marked as clean.
This commit adds a cluster state listener to mark the resource dirty
when a node is promoted to master. This will allow the new resource
to be published without any intervention.