testillano / http2comm Goto Github PK

Is your feature request related to a problem? Please describe.
The current design manages the maximum request body for every stream, regardless the server instantiated by the application.
This is because the value is an static member of stream class.

Describe the solution you'd like
Maximum request body size should be non-static and controlled under http2 server class.

Additional context
Memory reservation size will be affected by traffic managed for foreign servers, as currently it is stored in an static atomic variable.

Describe the bug
Delayed responses over broken connection crashes on response post.

To Reproduce
Provision a delay of 2 seconds and send a request with a smaller timeout (1 second).
When the connection is broke, the delayed event crashes.

Is your feature request related to a problem? Please describe.
Sometimes, library user don't need to process the request body received.
In such case, transforming a stringstream to std::string (via str()) is overkilling.
We will modify the interface to pass shared pointer to initial stringstream, and this could be
trnasformed on-demand later.

Also, we will calculate stringstream size withouth wasting time building str(). We will use tellg and seekg.

Is your feature request related to a problem? Please describe.
We would improve metrics capabilities adding dynamic counters for every different registered status code.
This must be done for both server and client endpoints.

Is your feature request related to a problem? Please describe.
Just an improvement to allow to provide a maximum value for number of threads, so the threads initially created could grow to a top value configured.

Describe the solution you'd like
Add a max value for worker threads which defaults to initial worker threads.
Http2Server interface adds a new size_t parameter, so it is non backward compatible.

Describe alternatives you've considered
Threads are not decreased when no longer needed, so be careful when configuring the max value.

Is your feature request related to a problem? Please describe.
Need access to a connection status description through client instance.

Is your feature request related to a problem? Please describe.
Add new virtual method to ignore request bodies

A new virtual method could be reimplemented to
ignore the request body storage under some
conditions, in order to speed up the performance
with high load.

This is useful when huge requests are received and not need to use their bodies.

Describe the solution you'd like
ignoreRequestBody, returns bool to ignore (false on base class default implementation).
Provide req structure to analyze.

Describe the bug
This server class, used as base class, has no virtual destructor.

Is your feature request related to a problem? Please describe.
This reception timestamp should be shared with the application user in order to better sinchronize this value with application events regarding latencies measured in this library metrics.

Describe the solution you'd like
Modify receive() at Http2Server class.

Is your feature request related to a problem? Please describe.
A monotonically increased sequence should be managed at this library, to simplify upper layers.

Describe the solution you'd like
Use an atomic uint64 and increment within on_data lambda

Is your feature request related to a problem? Please describe.
Timeout is poorly managed.

Describe the solution you'd like
We need to have an agreement to signal timeout event.
We need also to discard further receptions on timeout

Describe alternatives you've considered
Add a timed_out boolean flag to response task.

Family names never must e dynamic, so, class name is not an endpoint or something that could change (at least for clients).
Also, application/process name must not be provided in class name to be part of family name.
So, class name (fo1r Http2Server and Http2Client classes) shall be static. And the source label, which is dynamic, and could carry application/process name and client endpoints name, will be provided on enableMetricss() method.

Is your feature request related to a problem? Please describe.
The read timeout can be configurable in nghttp2 API.

Describe the solution you'd like
We will add this new parameter in the interface. Keep default value of 60 seconds as nghttp2 does.

Migrate deadline_timers to steady_timers:
https://lists.ozlabs.org/pipermail/openbmc/2022-March/029776.html

Also, migrate io_service to newer io_context when possible (nghttp2 still uses io_service) which is
modern and more efficient.

Reorganize metrics to better fit grafana expectatives. Redundant family name with "source" label to ease identification. Provide class name (which will be application name plus context information: module or http2 endpoint), to better configure externally.

Is your feature request related to a problem? Please describe.
Just to improve queue dispatcher monitoring, it would be interesting to know the
number of worker threads doing processing of data received.

Is your feature request related to a problem? Please describe.
Implement configurable pre reserve memory allocation for request body string container. This avoids possible reallocations done by std::string append() when the string object is constructed from scratch without initial memory reservation.

Describe the solution you'd like
The amount used for that reservation is the maximum size registered for incoming request bodies, something stored in static atomic member for Stream class.
The behavior will be configurable through virtual method preReserveRequestBody().

Is your feature request related to a problem? Please describe.
Method in 3rd party library is std::string but we use enum class.
So, we need conversions (takes time) and introduce complexity without need.

Describe the solution you'd like
Use std::string instead.

Use flag to know if timer is needed or not.

Is your feature request related to a problem? Please describe.
The current design increases monotonically the reception id for every stream, regardless the server instantiated by the application.
This is because the value is an static member of stream class.

Describe the solution you'd like
Reception id sequence should be non-static and controlled under http2 server class.

Additional context
Current approach is causing h2agent CT fail for this test:
FAILED server-data_operation/delete_data_test.py::test_001_i_want_to_delete_partial_internal_data_after_storing_some_traffic_events
This is because administrative flows are also increasing globally the sequence, but we want that sequence specific of each server.
Administrative server is not using that secuence, but also will be increased for every administrative request and only for them. Traffic requests only should increase traffic server sequence and admin traffic only should increase admin server sequence.

Is your feature request related to a problem? Please describe.
stringstream has worse performance than string, also we should provide to upper layer, a better usable interface. Const std::string reference is much better than a shared pointer to stringstream buffer.

Describe the bug
There is a tiny memory leak if delay responses are used for http2 server.

To Reproduce
long term traffic with a provision including delays in responses.

Implement a basic algorithm based in:

• number of busy threads
• size of FIFO queue dispatcher (when used)

Provide a maximum queue size to be allowed. Disabled by default (-1) this algorithm will activate the congestion situation when no idle consumers are available (busy threads are equal to consumers launched) and queue size is over the maximum provided.