Improve Local Mode #1230
Merged
Improve Local Mode #1230
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Local mode devices cannot handle more than 1 concurrent connection to the HTTP server. This means that if an additional connection is made when another one is pending or is in the process of being cleaned up by the device, it will return connection reset error. ==Mention of Failed Attempt== An approach where we acquire a lock and wait 100ms whenever we use the send() method was attempted but did not work consistently on all devices. My theory is that depending the on the processing on the device, it takes a different amount of time for it to reallocate resources to the next new connection. Therefore, simply retrying has worked just fine in my testing. It also has the added advantage of working despite another HA instance, app, etc. taking advantage of local mode. Signed-off-by: rany <ranygh@riseup.net>
Converting time.time() to an int and then converting that to ms risks resulting in the same sequence number for a given second. While we have safe guards against this already, it is better to prevent this scenario all together and just convert to ms properly. Signed-off-by: rany <ranygh@riseup.net>
Prevent the same sequence number being returned for different threads as this might cause hard to detect issues. Signed-off-by: rany <ranygh@riseup.net>
|
Thanks! Very impressive realisation |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Improve Local Mode Reliability
Local mode devices cannot handle more than 1 concurrent connection
to the HTTP server. This means that if an additional connection is
made when another one is pending or is in the process of being
cleaned up by the device, it will return connection reset error.
Mention of Failed Attempt
An approach where we acquire a lock and wait 100ms whenever we
use the send() method was attempted but did not work consistently
on all devices. My theory is that depending the on the processing
on the device, it takes a different amount of time for it to
reallocate resources to the next new connection.
Therefore, simply retrying has worked just fine in my testing.
It also has the added advantage of working despite another HA
instance, app, etc. taking advantage of local mode.
Use time.time_ns() to generate sequence number
Converting time.time() to an int and then converting that to ms
risks resulting in the same sequence number for a given second.
While we have safe guards against this already, it is better
to prevent this scenario all together and just convert to ms
properly.
Acquire lock when performing comparisons against/returning of sequence
Prevent the same sequence number being returned for different threads
as this might cause hard to detect issues.