Re: [libmicrohttpd] New Features: Create response from an IOVEC and Thread-safe Adding Connections to an EPoll Internal Thread MHD_Daemon

[Evgeny Grin]

Hello Damon and Lawrence,

Patches are always welcome, we're glad to extend MHD functionality.

Could you explain the new functionality added by 
0.9.72-internal_queue_connection.patch?
Probably you missed changes in the v0.9.72, but the queue of externally 
added connection is already implemented and used (see 
daemon->new_connections_head, new_connections_list_process_() and 
related). Looks like you partially re-implemented it (without the 
cleanup part). Have I missed some functionality in this patch?

About 0.9.72-MHD_create_response_from_iovec.patch:
* The header sys/uio.h is not portable. Many platforms don't have it so 
it cannot be used in main MHD header file (which should works on all 
platforms very straightforward). However you can define your own similar 
structures.
* You shielded implementation of send_iov_nontls(), send_iov_tls(), 
MHD_create_response_from_iovec(), and other functions by "#if 
defined(HAVE_SENDMSG) || defined(HAVE_WRITEV)" so MHD on platforms 
without sendmsg()/writev() will be built without this function. If you 
try to link application which is using MHD_create_response_from_iovec() 
on platform without sendmsg(), the linker will fail. This is not right, 
you need to implement an emulation for platforms lacking sendmsg(), like 
you did for TLS connection or similar.
There are other minor problems in your patch, I mentioned only the major 
ones.

Did you run any tests with/without your patch? Did it significantly 
increase throughput?

Please, use "diff -p" next time as it simplify reviewing the changes.

--
Best Wishes,
Evgeny

On 08.01.2021 20:14, Earp, Damon N. (GSFC-619.0)[SCIENCE SYSTEMS AND 
APPLICATIONS INC] via libmicrohttpd wrote:
> Afternoon,
>
> We've made changes to libmicrohttpd that we'd like to see integrated into the 
> project. If this isn't the appropriate forum for submitting patches please let 
> us know.
>
> I've attached the changes via three patch files, described below. All the 
> developed by my colleague Lawrence Sebald who is CC'd. We initially made these 
> changes to 0.9.62, and at the end of December ported them to 0.9.72. The 
> patched 0.9.62 has been in our production environment for over a month and 
> serviced billions of requests at this point. The patched version 0.9.72 is 
> almost done testing and is to be pushed to production later this month.
>
> Thanks,
> Damon Earp
>
> Attachments:
> 1. 0.9.72.patch
>      This contains all the changes that we've made to the library. I've 
> included this as it is machine generated, the other patch files are a manual 
> pruning of this file.
>
> 2. 0.9.72-MHD_create_response_from_iovec.patch
>      Added the ability to give MHD a vector of distinct buffers which make up 
> the response body. This allows a zero-copy for serving data that is fragmented 
> in memory. This change further adds the `MHD_IOVContentReaderFreeCallback` type 
> which includes a cls pointer on top of the iovec used to make the request.
>
> 3. 0.9.72-internal_queue_connection.patch
>      This is an ad hoc implementation to concurrently call 
> `MHD_add_connection()` on a daemon with no listen socket and an epoll internal 
> thread. We have been playing around with scheduling algorithms and have a pool 
> of `MHD_Daemon` workers which we can schedule a connection to. Our initial 
> attempts tried to rely on the locking in `internal_add_connection()` however we 
> ran into too many race conditions that we created a linked list of queued 
> connections in the daemon and then activated itc to drain the queue in the 
> internal thread.