Polling node rotation - #996
Conversation
Reworked the poller as the logic wasn't correctly rotating across snodes beyond on exception thrown, which caused issues when landing on a snode with no data which isn't an exception. Moved away from Promises in favour of coroutines
AL-Session
left a comment
AL-Session
left a comment
There was a problem hiding this comment.
Some suggestions only, accept or ignore as you see fit.
| } catch (e: Exception){ | ||
| Log.e(TAG, "Error while polling:", e) | ||
| pollDelay = minOf(MAX_RETRY_INTERVAL_MS, (RETRY_INTERVAL_MS * (NEXT_RETRY_MULTIPLIER * errorIncrement)).toLong()) | ||
| errorIncrement++ |
There was a problem hiding this comment.
| errorIncrement++ | |
| retryScalingFactor++ |
| if (rawResponse["code"] as? Int != 200) { | ||
| Log.e(TAG, "Batch sub-request had non-200 response code, returned code ${(rawResponse["code"] as? Int) ?: "[unknown]"}") | ||
|
|
||
| val responseList = (rawResponses["results"] as List<RawResponse>) |
There was a problem hiding this comment.
Are all these string literals "body", "results", code", "unknown" consts somewhere, or should they be?
There was a problem hiding this comment.
Added these to constants in the SnodeAPI, except for unknown which isn't a key here.
| namespace = Namespace.DEFAULT() | ||
| ), | ||
| auth = userAuth, | ||
| maxSize = -2) |
There was a problem hiding this comment.
Why -2? That's oddly specific... Comment would help?
There was a problem hiding this comment.
Please address this one, -2 is a psychotic "maxSize"
There was a problem hiding this comment.
Again the poller's logic itself hasn't changed so this is the existing logic. But we can do some digging.
In the definition here: https://github.com/oxen-io/oxen-storage-server/blob/dev/oxenss/rpc/client_rpc_endpoints.h#L193-L218
-2 is mentioned to represent half the response size, and was used as there is a limit on the size of the API response in the network.
Looking at the iOs code they seem to apply some fancy logic to determine this amount more dynamically based on the namespaces used, so we could likely optimise this at some point. Though the poller will at some point be centralised in libsession.
| publicKey = userAuth.accountId.hexString, | ||
| namespace = Namespace.DEFAULT() | ||
| ), | ||
| if (hashesToExtend.isNotEmpty()) { |
There was a problem hiding this comment.
Would it make sense to warn if it is empty? Merely asking.
There was a problem hiding this comment.
No, there is simply an action to take when it isn't
…ing_receiving/pollers/Poller.kt Co-authored-by: AL-Session <160798022+AL-Session@users.noreply.github.com>
3 participants
Reworked the poller as the logic wasn't correctly rotating across snodes beyond on exception thrown, which caused issues when landing on a snode with no data, which isn't an exception.
Moved away from Promises in favour of coroutines