Skip to content
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

perf tester: Count messages/handle channel close/listen backlog #70

Merged
merged 3 commits into from
Apr 7, 2022

Conversation

adamncasey
Copy link
Contributor

While testing #69 I found a few issues with the perf tester:

  1. A packet capture showed that we were sending two messages when it should only send one.
  2. Same capture showed that there was a shutdown issue due to amiquip sending channel close and us replying with channel openok - oops. The channel close doesn't seem to be sent since making the connection close explicit, but I left this fix in for future use.
  3. Changed the connection close to be explicit. But I am purposefully ignoring the result since sometimes this fails - which just seems to be when the server shuts the socket down before the client - when testing locally it seems like the client sends FIN first most of the time. I feel like the amqp library should probably ignore this case, but I'm also not sure how amqpprox handles this either so I probably can't be too upset - we do log a lot when connection are closing.

Noticed in a packet capture that we were sending two messages when
it should only send one.
Noticed during testing that amiquip was sending Channel Close but the server
send Channel OpenOk in return, triggering an error.

Since this happened while the connection was being Drop'ed the error seems
to have been swallowed.

The connection close is now explicit, even though this does avoid closing
the channel. I left in the channel close -> closeok in case we change something
here in future.
@adamncasey adamncasey merged commit c8bb8d2 into bloomberg:main Apr 7, 2022
@adamncasey adamncasey deleted the perftester branch April 7, 2022 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants