Skip to content

Relax log-level when polling engine is not defined#13882

Merged
mehrdada merged 1 commit intogrpc:v1.8.xfrom
mehrdada:relax-polling
Dec 30, 2017
Merged

Relax log-level when polling engine is not defined#13882
mehrdada merged 1 commit intogrpc:v1.8.xfrom
mehrdada:relax-polling

Conversation

@mehrdada
Copy link
Copy Markdown
Contributor

@mehrdada mehrdada commented Dec 28, 2017 •
edited by ncteisen
Loading

Fixes #13855
Fixes #13881
Fixes #13885

@mehrdada mehrdada requested review from sreecha and yang-g December 28, 2017 17:14
@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 28, 2017 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 28, 2017 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 28, 2017 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 28, 2017 

[microbenchmarks] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 28, 2017 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 28, 2017 

[microbenchmarks] No significant performance differences

@ctiller
Copy link
Copy Markdown
Member

ctiller commented Dec 28, 2017

I'd assert these log messages should be removed

@mehrdada
Copy link
Copy Markdown
Contributor Author

mehrdada commented Dec 28, 2017 •
edited
Loading

@ctiller These are only a subset of those introduced by #13281. Which ones, if any, of the ones introduced in that PR do you think should remain?

@ncteisen ncteisen mentioned this pull request Dec 29, 2017
Closed
@ctiller
Copy link
Copy Markdown
Member

ctiller commented Dec 29, 2017

If something is disabled by a compile time check, we should not log it.
If something is disabled by a runtime check, we should look it.

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 29, 2017 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 29, 2017 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Dec 29, 2017 

[microbenchmarks] No significant performance differences

@mehrdada mehrdada merged commit 77054fb into grpc:v1.8.x Dec 30, 2017
@mehrdada mehrdada deleted the relax-polling branch December 31, 2017 02:34
@apolcyn apolcyn mentioned this pull request Jan 8, 2018
Closed
@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@yang-g yang-g Awaiting requested review from yang-g

2 more reviewers

@sreecha sreecha sreecha approved these changes

@ncteisen ncteisen ncteisen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mehrdada @grpc-testing @ctiller @sreecha @ncteisen