Skip to content

benchmarks: Avoid sending a message after half close #2376

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.

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

Merged
merged 1 commit into from
Oct 26, 2016

Conversation

ejona86
Copy link
Member

@ejona86 ejona86 commented Oct 26, 2016

This doesn't impact test behavior per-se, but causes it to produce less
useless log output of the form:

java.lang.IllegalStateException: call was half-closed
    at com.google.common.base.Preconditions.checkState(Preconditions.java:174)
    at io.grpc.internal.ClientCallImpl.sendMessage(ClientCallImpl.java:380)
    at io.grpc.stub.ClientCalls$CallToStreamObserverAdapter.onNext(ClientCalls.java:299)
    at io.grpc.benchmarks.driver.LoadClient$AsyncPingPongWorker$1.onNext(LoadClient.java:406)
    at io.grpc.benchmarks.driver.LoadClient$AsyncPingPongWorker$1.onNext(LoadClient.java:400)
    at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onMessage(ClientCalls.java:382)
    at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1MessageRead.runInContext(ClientCallImpl.java:473)
    ... 7 more

Fixes #2372

This doesn't impact test behavior per-se, but causes it to produce less
useless log output of the form:

java.lang.IllegalStateException: call was half-closed
	at com.google.common.base.Preconditions.checkState(Preconditions.java:174)
	at io.grpc.internal.ClientCallImpl.sendMessage(ClientCallImpl.java:380)
	at io.grpc.stub.ClientCalls$CallToStreamObserverAdapter.onNext(ClientCalls.java:299)
	at io.grpc.benchmarks.driver.LoadClient$AsyncPingPongWorker$1.onNext(LoadClient.java:406)
	at io.grpc.benchmarks.driver.LoadClient$AsyncPingPongWorker$1.onNext(LoadClient.java:400)
	at io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onMessage(ClientCalls.java:382)
	at io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1MessageRead.runInContext(ClientCallImpl.java:473)
	... 7 more

Fixes grpc#2372
@ejona86 ejona86 force-pushed the de-error-loadclient branch from 1658b81 to f3ab4a4 Compare October 26, 2016 20:07
@ejona86
Copy link
Member Author

ejona86 commented Oct 26, 2016

@carl-mastrangelo, I pushed a cleaner version.

@@ -403,11 +403,13 @@ public void run() {
@Override
public void onNext(Messages.SimpleResponse value) {
delay(System.nanoTime() - now);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: won't now be wrong here?

Copy link
Member Author

Choose a reason for hiding this comment

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

onNext won't be called again. If it is, then an exception will be thrown because onCompleted is called more than once.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, I meant on the first time through

Copy link
Member Author

Choose a reason for hiding this comment

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

now is initialized to nanoTime(), and the first message is sent outside of the StreamObserver. So it looks like it is consistent.

@ejona86 ejona86 merged commit 98f4e41 into grpc:master Oct 26, 2016
@ejona86 ejona86 deleted the de-error-loadclient branch October 26, 2016 22:56
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Java QpsWorker produces many exceptions
2 participants