Skip to content

Comments

Fix logging of cluster state update descriptions#34182

Merged
DaveCTurner merged 2 commits intoelastic:masterfrom
DaveCTurner:2018-10-01-fix-update-descriptions
Oct 2, 2018
Merged

Fix logging of cluster state update descriptions#34182
DaveCTurner merged 2 commits intoelastic:masterfrom
DaveCTurner:2018-10-01-fix-update-descriptions

Conversation

@DaveCTurner
Copy link
Contributor

In #28941 we changed the computation of cluster state task descriptions but
this introduced a bug in which we only log the empty descriptions (rather than
the non-empty ones). This PR fixes that bug.

In elastic#28941 we changed the computation of cluster state task descriptions but
this introduced a bug in which we only log the empty descriptions (rather than
the non-empty ones). This PR fixes that.
@DaveCTurner DaveCTurner added >bug v7.0.0 :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. v6.5.0 v5.6.13 labels Oct 1, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@DaveCTurner
Copy link
Contributor Author

#28941 was justified by a microbenchmark that showed that this implementation was very significantly faster than the one that it replaces. Re-running this benchmark with this fix shows the improvement remains very significant.


Result "org.elasticsearch.benchmark.routing.allocation.AllocationBenchmark.measureStreamReduce":
  602.890 ±(99.9%) 15.975 ms/op [Average]
  (min, avg, max) = (562.935, 602.890, 666.777), stdev = 23.910
  CI (99.9%): [586.915, 618.864] (assumes normal distribution)

Result "org.elasticsearch.benchmark.routing.allocation.AllocationBenchmark.measureStringJoin":
  0.703 ±(99.9%) 0.011 ms/op [Average]
  (min, avg, max) = (0.668, 0.703, 0.736), stdev = 0.017
  CI (99.9%): [0.692, 0.715] (assumes normal distribution)

# Run complete. Total time: 00:02:17

Benchmark                                Mode  Cnt    Score    Error  Units
AllocationBenchmark.measureStreamReduce  avgt   30  602.890 ± 15.975  ms/op
AllocationBenchmark.measureStringJoin    avgt   30    0.703 ±  0.011  ms/op

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.

LGTM

@DaveCTurner
Copy link
Contributor Author

@elasticmachine test this please

Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

LGTM

@DaveCTurner DaveCTurner merged commit a7ce4b3 into elastic:master Oct 2, 2018
@DaveCTurner DaveCTurner deleted the 2018-10-01-fix-update-descriptions branch October 2, 2018 18:08
DaveCTurner added a commit that referenced this pull request Oct 2, 2018
In #28941 we changed the computation of cluster state task descriptions but
this introduced a bug in which we only log the empty descriptions (rather than
the non-empty ones). This change fixes that.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Oct 2, 2018
In elastic#28941 we changed the computation of cluster state task descriptions but
this introduced a bug in which we only log the empty descriptions (rather than
the non-empty ones). This change fixes that. Backport of elastic#34182.
DaveCTurner added a commit that referenced this pull request Oct 2, 2018
In #28941 we changed the computation of cluster state task descriptions but
this introduced a bug in which we only log the empty descriptions (rather than
the non-empty ones). This change fixes that. Backport of #34182.
kcm pushed a commit that referenced this pull request Oct 30, 2018
In #28941 we changed the computation of cluster state task descriptions but
this introduced a bug in which we only log the empty descriptions (rather than
the non-empty ones). This change fixes that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. v6.5.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants