Remove optimisations to reuse objects when applying a new ClusterState
#27317
Conversation
This file contains 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
DaveCTurner
added
:Cluster
:Distributed Coordination/Discovery-Plugins
bleskes
approved these changes
Nov 8, 2017
| @@ -154,4 +154,58 @@ public void testSnapshotDeletionsInProgressSerialization() throws Exception { | |||
| assertThat(stateAfterDiffs.custom(SnapshotDeletionsInProgress.TYPE), notNullValue()); | |||
| } | |||
|
|
|||
| public void testObjectReuseWhenApplyingClusterStateDiff() throws Exception { | |||
There was a problem hiding this comment.
while we're at it , can you check more than just the routing table and the index meta data? we have blocks, nodes and customs (out of the top of my head).
|
I added assertions for Blocks, DiscoveryNodes, and IndexTemplateMetaDatas. I tried Customs too but found no simple way to include them in the test without implementing local versions of the relevant abstract classes, and this ended up being more code than the test itself. I feel kinda comfortable with this, since Customs use the same basic machinery as the rest of them, so any issues are going to be hidden in a particular implementation, which is definitely out of scope for this test. |
|
(FAO @bleskes) |
DaveCTurner
added a commit
that referenced
this pull request
Nov 9, 2017
…ate` (#27317) In order to avoid churn when applying a new `ClusterState`, there are some checks that compare parts of the old and new states and, if equal, the new object is discarded and the old one reused. Since `ClusterState` updates are now largely diff-based, this code is unnecessary: applying a diff also reuses any old objects if unchanged. Moreover, the code compares the parts of the `ClusterState` using their `version()` values which is not guaranteed to be correct, because of a lack of consensus. This change removes this optimisation, and tests that objects are still reused as expected via the diff mechanism.
jasontedor
added a commit
that referenced
this pull request
Nov 9, 2017
* master: (22 commits) Update Tika version to 1.15 Aggregations: bucket_sort pipeline aggregation (#27152) Introduce templating support to timezone/locale in DateProcessor (#27089) Increase logging on qa:mixed-cluster tests Update to AWS SDK 1.11.223 (#27278) Improve error message for parse failures of completion fields (#27297) Ensure external refreshes will also refresh internal searcher to minimize segment creation (#27253) Remove optimisations to reuse objects when applying a new `ClusterState` (#27317) Decouple `ChannelFactory` from Tcp classes (#27286) Fix find remote when building BWC Remove colons from task and configuration names Add unreleased 5.6.5 version number testCreateSplitIndexToN: do not set `routing_partition_size` to >= `number_of_routing_shards` Snapshot/Restore: better handle incorrect chunk_size settings in FS repo (#26844) Add limits for ngram and shingle settings (#27211) (#27318) Correct comment in index shard test Roll translog generation on primary promotion ObjectParser: Replace IllegalStateException with ParsingException (#27302) scripted_metric _agg parameter disappears if params are provided (#27159) Update discovery-ec2.asciidoc ...
jasontedor
added a commit
that referenced
this pull request
Nov 9, 2017
* 6.x: Update Tika version to 1.15 Introduce templating support to timezone/locale in DateProcessor (#27089) Increase logging on qa:mixed-cluster tests Update to AWS SDK 1.11.223 (#27278) Improve error message for parse failures of completion fields (#27297) Remove optimisations to reuse objects when applying a new `ClusterState` (#27317) Decouple `ChannelFactory` from Tcp classes (#27286) Use PlainListenableActionFuture for CloseFuture (#26242) Fix find remote when building BWC Remove colons from task and configuration names Fix snapshot getting stuck in INIT state (#27214) Snapshot/Restore: better handle incorrect chunk_size settings in FS repo (#26844) Add unreleased 5.6.5 version number testCreateSplitIndexToN: do not set `routing_partition_size` to >= `number_of_routing_shards` Correct comment in index shard test Roll translog generation on primary promotion ObjectParser: Replace IllegalStateException with ParsingException (#27302) scripted_metric _agg parameter disappears if params are provided (#27159) Update discovery-ec2.asciidoc
clintongormley
added
:Distributed Indexing/Distributed
jpountz
removed
the
:Distributed Coordination/Discovery-Plugins
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.
In order to avoid churn when applying a new
ClusterState, there are some checks that compare parts of the old and new states and, if equal, the new object is discarded and the old one reused. SinceClusterStateupdates are now largely diff-based, this code is unnecessary: applying a diff also reuses any old objects if unchanged. Moreover, the code compares the parts of theClusterStateusing theirversion()values which is not guaranteed to be correct, because of a lack of consensus.This change removes this optimisation, and tests that objects are still reused as expected via the diff mechanism.