search

Antti / rust-amqp

AMQP client in pure rust. Corresponds to rabbitmq spec.
MIT License
249 stars 45 forks source link

Upgrade to OpenSSL 0.9.x #59

Closed grahamc closed 6 years ago

grahamc commented 6 years ago

Closes:

It isn't ideal, the 1s read timeout can slow things down a bit. But: It didn't take much to implement this, and lets me use a recent openssl. I'd be happy to fix it if you have any suggestions on how :)

Basically creates a new syncsender / reader for writes, and the "reading loop" also handles writes now.

This code is definitely POC. If you like it, I would like to clean it up for inclusion.

Antti commented 6 years ago

Thanks for the PR! I was trying to figure out if there was any way to upgrade this crate to support new openssl, and this approach was also considered. The problem with this is that it emulates async IO with timeouts, which is very resource consuming and slow (especially writes, since it needs to wait for read to timeout. And now I'm not sure that there is any easy way to upgrade to new openssl without doing an async rewrite of the client crate.

grahamc commented 6 years ago

I am also not sure :( Is that something you can / will take on? On Mon 13. Nov 2017 at 04:44, Andrii Dmytrenko notifications@github.com wrote:

Thanks for the PR! I was trying to figure out if there was any way to upgrade this crate to support new openssl, and this approach was also considered. The problem with this is that it emulates async IO with timeouts, which is very resource consuming and slow (especially writes, since it needs to wait for read to timeout. And now I'm not sure that there is any easy way to upgrade to new openssl without doing an async rewrite of the client crate.

— You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub https://github.com/Antti/rust-amqp/pull/59#issuecomment-343865079, or mute the thread https://github.com/notifications/unsubscribe-auth/AAErrOYKFkCrdcIwiwRbKCHU5HGygH_Pks5s2A9sgaJpZM4Qatob .

Antti commented 6 years ago

@grahamc I started rewriting this project using tokio, but at that time tokio was changing/breaking things every week or so and I stopped. You can have a look at this project lapin, which was initially written with async support in mind.

grahamc commented 6 years ago

Unfortunately Lapin and a Futures based mechanism is really not an appropriate approach for my use case, so I'm just fighting the losing battle of making this work. The latest commit will send many writes before attempting a read, this seems to make it more likely that a frame is ready to be read and seems to improve throughput.

grahamc commented 6 years ago

I'm going to close this PR since it is off my master branch, which is a bit junky and not really PR worthy. I've submitted PRs for non-SSL improvements out of this branch, though.