-
Notifications
You must be signed in to change notification settings - Fork 14
Conversation
6c54ea3
to
ecf0c56
Compare
ecf0c56
to
61ce84c
Compare
61ce84c
to
370a5a8
Compare
This reverts commit 5a40e72.
…lasdb into roomba/versions-props-latest
So as far as I can see tests are probably flaking because of this:
I wonder if it's this change somehow?https://github.com/palantir/dialogue/pull/1089/files |
These particular tests run through WireMock (I believe, but need to double check); conjure-java-runtime 5->6 bump replaces the feign okhttp client with dialogue. When the server unexpecteadly, prematurely closes a persistent connection, the dialogue http client will through a NoHttpResponseException: this should be retried (if we're not retrying it, we should); It kinda feels like the fact that some of these tests use WireMock proxy might be the smoking gun here. The hypothesis is that WireMock is killing all connections in a weird way and that blows through our retries? |
@carterkozak suggested that clearing the clients after killing the server should do it, and indeed it seems to have. I suspect jetty (dropwizard) does not cleanly close connections upon service stop? |
The java socket api doesn’t give us a way to tell if a socket is closed without doing a read or write, I expect the connections are being closed gracefully, we just don’t (can’t) notice until we try to use them. |
Yeah, I just tried bumping the graceful shutdown, that didn't work. |
I suppose not counting broken connections as a retry doesn't really work? |
It would be interesting to record the connection pool idle connection count when we get a nohttpresponseexception, if there are more pooled closed connections than retries, we’re doomed |
@sudiksha27 / @gmaretic can you approve? I have committed a workaround, and we'll see if we can make changes to dialogue, but I wouldn't want to block on that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for digging into this!
Released 0.293.0 |
excavator is a bot for automating changes across repositories.
Changes produced by the roomba/versions-props-latest check.
To enable or disable this check, please contact the maintainers of Excavator.