Increase robustness of LwsApiCall implementation #2134
Conversation
- Handle partial writes by sending data in multiple iterations - Use retries when message send fails - Track and handle message receive if in parts using `receiveMessage` var
vikramdattu
force-pushed
the
improve/lwsapicalls_robustness
branch
from
38a25f5 to
9f7ebd9
Compare
| retValue = (INT32) lws_write(wsi, pLwsCallInfo->sendBuffer + LWS_PRE, remainingSize, LWS_WRITE_TEXT); | ||
| if (retValue < 0) { | ||
| DLOGW("Write failed with %d", retValue); | ||
| CHK(FALSE, !STATUS_SUCCESS); |
There was a problem hiding this comment.
Should put an actual status here -- I believe this would get converted to 1 which is STATUS_NULL_ARG
| offset = ATOMIC_LOAD(&pSignalingClient->pOngoingCallInfo->sendOffset); | ||
| size = ATOMIC_LOAD(&pSignalingClient->pOngoingCallInfo->sendBufferSize); | ||
|
|
||
| result = (SERVICE_CALL_RESULT) ATOMIC_LOAD(&pSignalingClient->messageResult); | ||
|
|
||
| if (offset != size && result == SERVICE_CALL_RESULT_NOT_SET) { | ||
| CHK_STATUS(CVAR_WAIT(pSignalingClient->sendCvar, pSignalingClient->sendLock, SIGNALING_SEND_TIMEOUT)); | ||
| retryCount++; |
There was a problem hiding this comment.
Wondering if we can debug log (DLOGD) the retry count increased
There was a problem hiding this comment.
If the CVAR_WAIT returned non-success (eg status operation timed out), CHK_STATUS would goto Cleanup and bypass the retry count -- doesn't seem intended since there's a "// Check if we timed out" down below
| connectInfo.port = SIGNALING_DEFAULT_SSL_PORT; | ||
| connectInfo.alpn = "http/1.1"; // Force HTTP/1.1 only | ||
| connectInfo.protocol = "http/1.1"; // Force HTTP/1.1 protocol |
There was a problem hiding this comment.
Wondering the purpose of the H2 flag if 1.1 is forced, is it for latency/performance optimization?
It seems that HTTP 2 should from this quick test:
curl -sI https://m-xxxxxxxx.kinesisvideo.us-west-2.amazonaws.com -o/dev/null -w '%{http_version}\n'
2
curl -sI https://v-xxxxxxxx.kinesisvideo.us-west-2.amazonaws.com -o/dev/null -w '%{http_version}\n'
2
| @@ -1907,6 +1948,9 @@ STATUS writeLwsData(PSignalingClient pSignalingClient, BOOL awaitForResponse) | |||
| SIZE_T offset, size; | |||
| SERVICE_CALL_RESULT result; | |||
|
|
|||
| UINT32 retryCount = 0; | |||
| const UINT32 MAX_RETRY_COUNT = 3; | |||
There was a problem hiding this comment.
Thinking this should be a #define (very very minor memory reduction)
receiveMessagevar