CodeCompass icon indicating copy to clipboard operation
CodeCompass copied to clipboard

Update Mongoose to newest release version (6.18)

Open whisperity opened this issue 5 years ago • 7 comments

The Mongoose library major version we were using (5.4) is from 2014 and is no longer actively developed. There is a newer version of the 6.y line, however, moving to this version required a significant rewrite of the threaded response handling.

Originally my first hunch of implementing SSL for #387 was to update Mongoose and try, I only afterwards found out that 5.4 also supports SSL.

The previous behaviour was that we were running on each thread a Mongoose server that was listening on the same (manually injected) socket. This behaviour is not supported any more, the new Mongoose version threw away "real" multithreading support, and the API no longer allows injecting the socket. (In addition, I was told that the same process on n threads listening on the same socket resulted in the kernel waking up every thread whenever a request was received.)

To align the API with the new version, a different approach is used: incoming HTTP requests, if are of trivial nature (static file access, which will also help for this patch to be joined together with #386!) are served as if Mongoose was serving a static directory - there were also massive API changes here. Otherwise, the request will be saved into a thread pool's queue, similar to the one used by the parser engine, and handled by dispatching to the Thrift endpoints.

The indicators MG_FALSE and MG_TRUE are also gone, the new API requires for the user of the library to manually handle doing A or B when an URL is accessed. Previously Mongoose got to know that the callback didn't do anything with the request and fell back by itself to serving the static resource.

The upside of this is that there is a dedicated thread for listening on the socket, and as such, heavy-handed operations will not lock static page accesses from being served. Of course, the number of threads for the service workers is an upper limit to how much Thrift requests can be served at a time.

Most of the changes in this patch are of a highly technical nature:

  • Need to bind the listener with new API.
  • The request handler callback (from Mongoose's point of view) does not directly handle the request, and as such, returning from it would destroy the request buffer, so we have to side-step this issue by saving the important data out.
  • Performing the same decoupling on the handlers' API.

whisperity avatar May 01 '20 19:05 whisperity

Rebased over #386.

whisperity avatar May 11 '20 10:05 whisperity

With this patch the page load and every network request take a few seconds. Everything became slow, so this needs some debugging. I'll try to investigate too. Apart from this, the PR seems to be fine.

I can verify @bruntib 's observation, with this PR, a HTTP request towards e.g. ProjectService takes more than 1 sec. Previously it was around 4-7 ms, so it got slower about 150-250 times, which cannot be a normal functionality. Tested on localhost, in Release build.

mcserep avatar Jul 25 '20 14:07 mcserep

Huh, weird, I haven't measured a difference when I made this. Well, one reason could be, is that the handling of the request is a blocking operation because Mongoose no longer allows on the API to share the listening socket between individual threads...

I am, unfortunately, out of ideas, then. Apart from sticking to the old Mongoose ad infinitum.

whisperity avatar Jul 25 '20 19:07 whisperity

@whisperity I don't think it is a threading (blocking) issue, since I have measured it on a single HTTP request, making sure that there were no other requests. (Also if Moongose's performance would be this poor under normal usage, it would render it unusable, so my guess is that we are doing something not correctly.)

It would be nice to update Moongose, maybe we can execute a performance test on CodeCompass_webserver (e.g. with CLion and perf) and check where does it spends so much time when handling a request.

mcserep avatar Jul 25 '20 19:07 mcserep

I'm leaving the patch up, but I do not think I'll have the mental capacity and acuity to reverse engineer and dissect this patch in the foreseeable future.

whisperity avatar Jul 26 '20 10:07 whisperity

I'm leaving the patch up, but I do not think I'll have the mental capacity and acuity to reverse engineer and dissect this patch in the foreseeable future.

Okay, I might add some information meanwhile if I get to debug this. Right now the addition I can make is that static files are served normally, only our services are served slowly.

mcserep avatar Jul 28 '20 11:07 mcserep

What is the status of this PR, @whisperity ? Do you think you will have the time to get back on this issue anytime in the future? It would be really nice to upgrade the obsolete Mongoose version we are using. As I have checked, version 7.x came out December 2020, and we still use version 5.4.

mcserep avatar May 08 '21 19:05 mcserep

I have updated this development to be sync with the current master: https://github.com/mcserep/CodeCompass/tree/update-mongoose

If you enable in the PR settings to "Allow edits by maintainers", then I can update it on your fork, @whisperity.

More importantly, I have made progress towards the reason of the slow page load time of the Mongoose 6 refactored webserver. The key is the timeout parameter for mg_mgr_poll().

void MongooseServer::loop()
{
  // Trap termination so the server stops.
  _detail::SignalHandler trapSigint(SIGINT, MongooseServer::signalHandler);
  _detail::SignalHandler trapSigterm(SIGTERM, MongooseServer::signalHandler);

  _loopThreadId = std::this_thread::get_id();

  while (!exitCode)
    mg_mgr_poll(_mongoose.get(), 1000);
}

With the value 1000, all (non-statically served) requests take at least 1000 milliseconds. If I set the parameter to 20, then requests are served in 24-26 ms. If I set the parameter to 10, then requests are served in 14-16 ms. However, if I set the parameter to 0, then some requests are failed to be served.

This timeout parameter of Mongoose seems to work strange.

mcserep avatar Feb 11 '23 01:02 mcserep

Apart from this, the webserver does not seem to serve requests in parallel - at least for Thrift requests, where it was easily observable with the high page load time.

Is there any reason we are not using the built-in multithreaded support of Mongoose? (MG_ENABLE_THREADS and mg_start_thread)

mcserep avatar Feb 11 '23 01:02 mcserep

I close this PR, as there have been no real development on this in the past 3+ years.

This could be reopened if the discussed issues are resolved; or since Mongoose 7.x is the current major release, maybe a new PR could be made in the future, updating the webserver to that version.

mcserep avatar Feb 05 '24 07:02 mcserep