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

Switched to using the uWebSockets library for http. #364

Closed

Conversation

floriankramer
Copy link
Member

We are currently using a custom http implementation on top of native sockets to provide the http server. This pr instead uses
the uWebSockets library to provide the http and routing implementation. The library was chosen for its advertised speed, being maintained and having a small list of dependencies (currently only zlib. We also need openssl for it, if we want ssl support).

I've currently done the bare minimum to get the library working, as it wasn't quite clear yet what library we want to use, and I was mainly curious about the usability of the library. There is a lot of old code that can be removed, and the integration into cmake should probably be changed (e.g. by using ExternalProject_Add).

@joka921 @hannahbast What do you think about the library? Should I finish this pr, or do we want to use something else?

@joka921
Copy link
Member

joka921 commented Nov 27, 2020

Thank you very much. I really like this so far.
I think for the Server business we need external dependencies, since the current implementation by POSIX-Sockets
is very hard to maintain and hacky.
The library you found also seems to be reasonable (small, fast, compatible licensing and well maintained).
Also This would help very much by indeed adding a Websocket API which I have somewhere on my list.
This would allow tracking the progress of queries in the UI or canceling queries etc.

@hannahbast Could you also comment on what you think about external HTTP libraries in general and this one in particular?

@niklas88
Copy link
Member

I also think this is a very good idea, I originally planned to use Beast HTTP that's currently being incubated for the boost project. Used that on another test, but yeah anything decently maintained will be leaps better than what's being used now, so I'm all for it!

@joka921
Copy link
Member

joka921 commented Nov 29, 2020

I have also looked a tiny little bit into BEAST but I prefer this solution for the following reasons:

  • its lighter to integrate (BEAST basically requires all of Boost)
  • it seems much easier to use (the BEAST examples seemed to me as if you still had to do a lot of low-level things manually).

@niklas88
Copy link
Member

Ok, when I tried it it was still possible to use beast without the rest of boost and even a modified (i.e. don't spawn a thread per request) classic threaded model. IMHO that still fits QLever well since result generation does need CPU and so one thread per CPU with a shared accept() is both simple and also makes sure the system doesn't get overloaded (just last week spawning a thread per connection killed almost the enitre AWS us-east-1).

There should be a version of a wrapper for that still in the AD Gitlab but yeah I think Beast is definitely adopting the whole Boost stuff. So I think uWebSockets makes sense and if I skimmed the code right it would even still use the same threading model, though it's a bit hard to tell what that listen call does.

floriankramer and others added 3 commits November 17, 2021 09:55
Switched to using the ExternalProject module for using uWebSockets makefile.

Added several listening threads.

Server.cpp code style

Switched to the FindOpenssl module for linking open ssl.

Added openssl libraries to the dockerfile

removed one line of whitespace to tests a workflow
Ran clang-format after the rebase.
Copy link
Member

@hannahbast hannahbast left a comment

Choose a reason for hiding this comment

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

1-1 with Johannes, with some notes

@joka921
Copy link
Member

joka921 commented Dec 7, 2021

Thank your very much for this work.
In the meantime we have decided to not use uWebSockets for several reasons, but are using a server based on Boost::Beast.
Note that this does not speak against your code (which is of high quality, as usual), but against the underlying library.

@joka921 joka921 closed this Dec 7, 2021
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.

None yet

4 participants