QuantumEntangledAndy / neolink

An RTSP bridge to Reolink IP cameras
GNU Affero General Public License v3.0
257 stars 41 forks source link

Fix registering death spiral #122

Closed sqeeswy closed 11 months ago

sqeeswy commented 11 months ago

@QuantumEntangledAndy thanks a lot for Your work on this fork.

When trying to register for the relay and failing we will most likely select the same relay for the next registration. This avoids this situation by removing the relays with failing registrations from the pool for current try.

This resolves issues with relay discovery for some 4g cameras that are in other geolocations then our server. We were constantly getting the closest of the relays (because it was the first one to get response from during lookup) but if registering failed we entered registering death spiral even if for other relays we could get connection.

Most likely resolves #121

QuantumEntangledAndy commented 11 months ago

Perhaps you can explain what you mean by death sprial. The code as written already dosent stop on the first message back. It keeps waiting for the first successful one (the one that succeds the function callback). All relays are tried in async and we disregard failures and just wait for either a success or all failures

QuantumEntangledAndy commented 11 months ago

Does the first register return a non zero port etc but is still in fact invalid?

If that's the case I might prefer one call to all registers grabbing all register results ONCE then looping over every valid result. It should use less calls and be faster then a retry of all remaining ones from the beginning.

QuantumEntangledAndy commented 11 months ago

Or perhaps an async map after we get the result from one where we try it while the others keep waiting for their results. That should be even more efficient and should be doable with an async iterator.

QuantumEntangledAndy commented 11 months ago

Ok so I had ago at this myself. Where I send a request to ALL reolink servers async.

As the async replies become available I then use those results to register the address. If it succeeds I take that result as the register. If not I keep waiting for the next async reply. If all replies fail to register then report failure.

Could you check it out on my fix/async_lookup_and_register branch?

sqeeswy commented 11 months ago

Does the first register return a non zero port etc but is still in fact invalid?

That's exactly what is happening.

Or perhaps an async map after we get the result from one where we try it while the others keep waiting for their results. That should be even more efficient and should be doable with an async iterator.

This looks like the best option, my solution was indeed not great from optimization standpoint.

Ok so I had ago at this myself. Where I send a request to ALL reolink servers async.

As the async replies become available I then use those results to register the address. If it succeeds I take that result as the register. If not I keep waiting for the next async reply. If all replies fail to register then report failure.

Could you check it out on my fix/async_lookup_and_register branch?

Thanks a lot for that, I will check it as soon as I will be able.

QuantumEntangledAndy commented 11 months ago

Closing this as it is addressed in #123 and #125 instead.