Skip to content

Update Envoy & HdrHistogram_c #561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Oct 12, 2020
Merged

Update Envoy & HdrHistogram_c #561

merged 5 commits into from
Oct 12, 2020

Conversation

oschaaf
Copy link
Member

@oschaaf oschaaf commented Oct 12, 2020 •
edited
Loading

  • Update Envoy to 2097fe908f2abb718757dbd4087d793c861d7c5a
  • Update HdrHistogram_c to 0.11.2

Adds --define tcmalloc=gperftools to the opt build we use to push images,
to (continue to) allow cpu profiling. We need this because of envoyproxy/envoy@4c0d2d2

Signed-off-by: Otto van der Schaaf oschaaf@we-amp.com

@oschaaf
Update Envoy & HdrHistogram_c
8ced444 
- Update Envoy to 2097fe908f2abb718757dbd4087d793c861d7c5a
- Update HdrHistogram_c to 0.11.2

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@oschaaf oschaaf added the waiting-for-review A PR waiting for a review. label Oct 12, 2020
@oschaaf
Use --define tcmalloc=gperftools to continue to allow profiling.
f09af35 
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@oschaaf
Add --define tcmalloc=gperftools in CI benchmark task
11eb719 
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
dubious90
dubious90 previously approved these changes Oct 12, 2020
View reviewed changes
@dubious90 dubious90 self-assigned this Oct 12, 2020
@dubious90 dubious90 added waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. and removed waiting-for-review A PR waiting for a review. labels Oct 12, 2020
@dubious90
Copy link
Contributor

Can you resolve the clang tidy failure, and then re-apply the waiting-for-review label?

@oschaaf
Clang-tidy: fix script. Fix actual complaint that now emerges.
a128bfe 
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@oschaaf
Copy link
Member Author

oschaaf commented Oct 12, 2020

@dubious90 a128bfe addresses the clang tidy issue here. In this PR, that task started to structurally fail without showing an actual complaint (it started flaking earlier, which is tracked in #546).
I copied over changes from Envoy's version of the clang tidy check script to our own, and after doing so an actual complaint emerged that I addressed. As a side effect, I suspect #546 will also be resolved upon merging this.

dubious90
dubious90 previously approved these changes Oct 12, 2020
View reviewed changes
@oschaaf
Fix last quotation issue
dd0e198 
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@oschaaf
Copy link
Member Author

oschaaf commented Oct 12, 2020

@dubious90 sorry - with dd0e198, which resolves a last quotation issue this now passes. I'd like to retry a few times to see if this also deflakes the clang-tidy task in CI, I'll ping you on Slack once I know more about that.

@oschaaf oschaaf added waiting-for-review A PR waiting for a review. and removed waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. labels Oct 12, 2020
@oschaaf
Copy link
Member Author

oschaaf commented Oct 12, 2020

@dubious90 unfortunately, this doesn't resolve the clang-tidy flakes, though it is an enhancement and now passes again some of the time instead of structurally failing. resolving #546 needs more looking into.

@dubious90 dubious90 merged commit 3eea4b5 into envoyproxy:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@dubious90 dubious90 dubious90 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@dubious90 dubious90

Labels
waiting-for-review A PR waiting for a review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@oschaaf @dubious90