search

Dushistov / couchbase-lite-rust

Lightweight, embedded, syncable NoSQL database engine wrapper for Rust
Apache License 2.0
22 stars 10 forks source link

Tokio spawn issue for websocket impl #94

Closed CedricCouton closed 2 years ago

CedricCouton commented 2 years ago

Hi,

Regularly in our production we have some log that indicate that the socket is closed when the ws_write try to write in the socket. After analyse the log of the sync gateway we have two cases : invalid checksum and error during decompression. I focused on the first one to start. Invalid checksum is throw by the blip protocol. Checksum is cumulative so the value depends of all the precedent messages. When I deep dive in the log I can see that there is sometimes missing messages (MSG#) : bump from msg#6 to msg#7 or 8 for example. When I tried to reproduce I didn't success to. It's a very edge case. After some loop + logs I finally reproduced it and figured out the reason. In the ws_write function the Tokyo::spawn is used but it don't guarantee the order of the execution of the future spawned. so sometimes we can have the bad order of messages in case of multithreaded executor of tokio runtime. I think we can use here a block_on wdyt? I will submit a pr for that soon. Cheers

Dushistov commented 2 years ago

I think we can use here a block_on wdyt?

block_on has many downsides, it panics if called during runtime shutdown, and has real issues with single thread runtime.

CedricCouton commented 2 years ago

Yes no doubt about that, it's just a proposition. How can we solve the issue :-) ? a runtime of one worker thread? like that : 

tokio::runtime::Builder::new_multi_thread().worker_threads(1).enable_all().build()
CedricCouton commented 2 years ago

Maybe we can have a queue adding the message inside it and pop a message in each spawned future?

Dushistov commented 2 years ago

Maybe we can have a queue adding the message inside it and pop a message in each spawned future?

Or we can spawn one task during socket open that read channel and write to socket in the loop. May be reuse main_read_loop for that. There is only one thing that tricky is properly handle closing. Because of we need send close ack.

CedricCouton commented 2 years ago

Yes it's a good idea. But how we can write in the channel that have to be a Tokio channel, in a future so, in the good order :)?

Dushistov commented 2 years ago

But how we can write in the channel that have to be a Tokio channel, in a future s

I meant that writing to channel happens in sync code, not in the future: https://docs.rs/tokio/1.20.0/tokio/sync/mpsc/struct.UnboundedSender.html#method.send