Surreal-Net / Surreal.Net

Database driver for SurrealDB available for REST and RPC sessions.
Apache License 2.0
57 stars 7 forks source link

Added test for retrieving large documents #104

Closed Du-z closed 2 years ago

Du-z commented 2 years ago

Note: this PR make use of changes from #103, merge that first.

The RPC test hangs and must be forcefully killed if the variable count is increased past 1 where the retrieved document size surpasses the buffer size defined in WsTx.DefaultBufferSize.

@ProphetLamb I have not been able to figure this one out yet, I'm hoping you might have some insights.

Some notes from my debugging: The rest client works normally. System.Text.Json has it's own DefaultBufferSize which also defaults to 16 * 1024

ProphetLamb commented 2 years ago

The issue is related to communicating the next block expectations from the client to the server. The client expects one more block, but states it already consumed the entire message, even tho there is still stuff in the buffer, which is discarded. So we wait for our buffer to fill, from a discarded buffer. Which is impossible, thus we have a soft deadlock

Du-z commented 2 years ago

I think I have found the issue.

WsStream.ReadAsync(Memory<byte> buffer, CancellationToken cancellationToken = default) needs to consume the prefix first before it starts to fill the buffer from the websocket.

The test no longer locks. when I debug the tests (even with no breakpoints) the test it goes green, but running it normally I get a JsonException that is on the boundary of a buffer.

System.Text.Json.JsonException
':' is an invalid end of a number. Expected a delimiter. Path: $ | LineNumber: 0 | BytePositionInLine: 16385.

See 97d5b72 for my attempt.

Du-z commented 2 years ago

@ProphetLamb It looks like Ws.Receive() was stealing parts of the stream that was still being read by WsStream.

This is because WsTx.Tr() returns to Ws.Receive() before the created WsStream has finished asynchronously reading.

My solution is very hacky and really just a proof of concept. Is there a better way to block Ws.Receive() until the stream has completed?

(The remaining error is fixed in #107 )