no keep alive support?

gloomyandy

I assume that using keepalive would mean that the http connections to the server would have a longer lifetime. One potential problem with this may be that RRF can only support a relatively small number of connections (and that some versions like our LPC port the number is lower still). Increasing the lifetime of these connections may make it more difficult to support additional clients.

At the moment due to the short life of a connection it is possible for several clients to share the small pool of connections (with perhaps some delay) even though over some period of time there may be more total connections than there are available server sockets. However if the lifetime of those connections is longer this sharing may no longer be possible.

I would ask that if this feature is introduced that the server should be able to refuse to allow it (hopefully in a way that does not cause problems for the client side) and that client code should not depend upon it.

DanS79

@gloomyandy on most servers you can enable/disable it. You can alsoalso set a timeout in seconds. for example if the timeout is set to 1 second., and a client isn't sending at least 1 request per second, the connection gets closed and the client will ned to open a new one.

take a look at this.
https://blog.catchpoint.com/2010/09/17/anatomyhttp/
what we have now is a "Simple HTTP Transaction" what would be better imo, is "Persistent HTTP Transactions"

keep in mind thats right now dwc is doing several Simple HTTP Transactions per second per client depending on what you have the update interval set to.

gloomyandy

In some situations the firmware will keep the connection open if the client requests it. In particular when sending Json responses it does this. I'm not sure if DWC makes use of this feature but I suspect it may for some of the more frequent operations. (for more details see: https://github.com/Duet3D/RepRapFirmware/blob/dev/src/Networking/HttpResponder.cpp#L448).

It is worth remembering that this is not "most servers" in the stand alone case the server is running on a small mcu with (in the case of the LPC) only 64Kb of RAM. So it's not just a case of turning an option on, changes will be needed in the code (see above) and as you can see from some of the existing comments juggling with memory buffers is already fairly complex.

As I mentioned previously in the case of the LPC we can only support 2 sockets open at once. So keeping both of those open for a second (just in case) will mean that if any other client attempts to open a connection it may be refused, or at best will have to wait for that one second to timeout. In the worst case (if the client keeps sending requests) the socket may never get closed and so other clients will be unable to connect. Things are a little better on the Duets, but they only have a relatively small number of available sockets.

I'm not saying don't do it (though I think the consequences need to be worked through), I'm just asking that this is not made a requirement (either explicitly or through a particular level of performance being required). Because I suspect that doing so will move the LPC port into being unusable, which will probably mean that someone will fork DWC (with a consequent reduction in testing of the Duet version which I think does have some value for Duet users).

In the case of the SBC version of course things are very different as that does not have to deal with the same limitations.

I realise this may not be important to the majority, but I just wanted to raise the issue (and you did ask for comments).

chrishamm

That feature isn't desirable with RRF in standalone mode because we have some boards that only support up to 8 connnections where half of them can be allocated to non-HTTP services. That leaves us with potentially only 4 sockets for serving DWC, and we don't want to block them very long. So there hasn't been much anticipation in implementing keep-alive support in the HTTP responders either. It's really a similiar restriction to the LPC port that @gloomyandy explained.

For the same reason DWC is compiled with as few JS chunks as possible.

DanS79

@gloomyandy said in no keep alive support?:

I'm not sure if DWC makes use of this feature but I suspect it may for some of the more frequent operations.

DWC/Chrome is asking for it. request header "Connection: keep-alive"
however the server always responds with a response header "Connection: close".

I have yet to see it allow keep-alive.

(for more details see: https://github.com/Duet3D/RepRapFirmware/blob/dev/src/Networking/HttpResponder.cpp#L448).

If i read that properly the code already has everything it needs to handle persistent connections, as it's inspecting the request headers in HttpResponder::SendJsonResponse

see line 986 through 999
https://github.com/Duet3D/RepRapFirmware/blob/dev/src/Networking/HttpResponder.cpp#L986

however when HttpResponder::SendJsonResponse calls HttpResponder::GetJsonResponse the first thing it does it set keepOpen = false;

https://github.com/Duet3D/RepRapFirmware/blob/dev/src/Networking/HttpResponder.cpp#L450

so the if statement that starts on 987 never gets run.

assuming i'm reading this properly, I haven't worked with c/c++ in almost 20 years.

Zambiorix

Hello All,

This a an old thread, I know, but I would love to see the keep-alive feature supported for rest-api in future firmware updates.
(https://github.com/Duet3D/RepRapFirmware/wiki/HTTP-requests)

I understand that keeping sockets open has a resource impact. But maybe it can be up to the user to decide what is important in a particular situation? And frequent polling has a resource impact too...

For my project I am using a Duet 3 MB6HC v1.0 standalone and I do not use the DWC at all (updates via bossa). I am developing specific controller software and rely on good and fast socket connections.

So, maybe this feature can be enabled via a setting, for those who would benefit from it?

Ideally, I would love a websocket, so that I do not have to poll anymore. But keep-alive is a great alternative. Possibly with a (configurable) timeout of 1 to a few seconds?

Hopefully you would reconsider

Thanks

Gerd

chrishamm

@Zambiorix I think both features are already supported in SBC mode and v3.5 will get an emulation layer for the rr_ style HTTP API as well (see here for the DSF RESTful API). In addition, DSF provides an IPC socket, so you may not even need to use HTTP if go that way.

~~We don't have plans to add keep-alive support to the firmware anytime soon and since we're short on sockets in standalone mode, I don't think we will add WebSocket support to the firmware either.~~ WebSockets can be used to receive object model updates in SBC mode, though.

Zambiorix

@chrishamm

I am not using duet3 in SBC mode, I am developing my own custom controller and want to remove layers of complexity, if possible.

Looking at the code, keep-alive is implemented, at least partially, in

src/Networking/HttpResponder.cpp

I have found several lines where Connection is set to close and other lines where a keepOpen variable is used to set close or keep-alive.

I also found a line where the socket is kept open

Commit(keepOpen ? ResponderState::reading : ResponderState::free, false);

And other places close it straight away

Commit(ResponderState::free, false);
//or
Commit();

In GetJsonResponse I have noticed, keepOpen is set to false and never changes.

bool HttpResponder::GetJsonResponse(const char *_ecv_array request, OutputBuffer *&response, bool& keepOpen) noexcept
{
	keepOpen = false;	// assume we don't want to persist the connection
...

To me all this looks like 90% of the keep alive support is already in place?
I would try to finish the implementation myself.

I could check for the existence of an extra custom header and, if present, keep the socket open. This would prevent browsers and other connections that send keep-alive from actually keeping the socket open.

The HttpReceiveTimeout is set to 2 seconds, so the socket will close automatically when not used.

I only need 1 connection for my project, so I could also try to implement that only 1 socket can be kept alive concurrently, the rest is ignored.

Do you know of any other pitfalls?
Keep-alive apparently has been tested in the past?

Thanks
Cheers

Gerd

chrishamm

@Zambiorix OK, so in fact it looks like it is already supported in the firmware. If a client sends a JSON request with Connection: keep-alive in the headers, it should keep the connection alive already. What board (base) are you using?

Zambiorix

@chrishamm

I am using a Duet 3 MB6HC v1.0

There is some support implemented, but not for every command.
(text/plain commands don't have it)

And currently keep-alive is disabled, so Connection header is always set to close.

It has been tested in the past, but not sure it has only been disabled due to resource use ... there could still be other issues.

I would allow keep-alive for following conditions:
#1: Connection request header is set to keep-alive
#2: a custom request header is present (so browsers and other connections that set keep-alive by default are locked out)
#3: only one keep-alive socket is active concurrently
#4: read timeout of 2 seconds is plenty enough to automatically close the socket

This way there is no impact for default use.

At the moment I have no development environment installed and I don't have much time right now, but I would definitely look into this in the future.

Zambiorix

@chrishamm

I have installed the development environment and implemented keep-alive for HttpResponder.

Its has no impact on browser calls, those sockets get closed even when Connection: keep-alive is set.

If the Rrf-Keep-Alive: force header is set, keep-alive starts working for a single socket. (so max 1 concurrent open socket)
__LPC17xx__ is also checked and, if set, disables keep-alive.

I have to do some more testing, but all seems to work smooth.

If interested I can create a pull request, for you to verify?

Cheers
Gerd

chrishamm

@Zambiorix Yes, sounds good, thanks. Please target the 3.5-dev branch if possible.

Zambiorix

@chrishamm

When moving to the 3.5-dev branch, I get the following build error:
unrecognized option '--no-warn-rwx-segment'

I'm using ARM toolchain version 10.3-2021.10

Should I use a different one?

Thanks
Cheers

Gerd

gloomyandy

@Zambiorix See: https://github.com/Duet3D/RepRapFirmware/wiki/Building-RepRapFirmware, if you are using a current 3.5-dev you are in effect working with 3.5 beta 3.

Zambiorix

@chrishamm

Ok, I had to install a more recent ARM toolchain (12.2) and after changing ArmGccPath and adding CrcAppender to the PATH, it builds.
But on generating the binary file, I get:

Generating binary file
arm-none-eabi-objcopy -O binary "/Users/gerd/Documents/Projects/Projects/Duet3D/RepRapFirmware/Duet3_MB6HC/Duet3Firmware_MB6HC.elf" "/Users/gerd/Documents/Projects/Projects/Duet3D/RepRapFirmware/Duet3_MB6HC/Duet3Firmware_MB6HC.bin" && CrcAppender "/Users/gerd/Documents/Projects/Projects/Duet3D/RepRapFirmware/Duet3_MB6HC/Duet3Firmware_MB6HC.bin"
Failure processing application bundle.
Bundle header version compatibility check failed.
A fatal error occured while processing application bundle
make[1]: [post-build] Error 159 (ignored)

any ideas?

Zambiorix

@gloomyandy thanks! Figured out I had to install a more recent arm toolchain