knowm / XChange

XChange is a Java library providing a streamlined API for interacting with 60+ Bitcoin and Altcoin exchanges providing a consistent interface for trading and accessing market data.
http://knowm.org/open-source/xchange/
MIT License
3.87k stars 1.94k forks source link

[Bitmex Stream] Issue with orderbook #3635

Open conspyrosy opened 4 years ago

conspyrosy commented 4 years ago

I've noticed the Bitmex orderbook displays different quotes from the UI. Not sure what's causing this but creating this issue to track. Will have a look at it today hopefully

Screen Shot 2020-08-01 at 09 32 51
conspyrosy commented 4 years ago

This is more noticeable when prices are moving fast - when prices are stable I didn't notice the issue.

Couldn't find out what's wrong - the code looks sound. Will come back to this at some point.

badgerwithagun commented 4 years ago

Most likely this is a concurrency issue. We use two Netty event loop threads per exchange which means that it's possible for two events to get processed concurrently. A lot of implementations don't take account of this and use non-threadsafe collections types to hold the snapshot.

conspyrosy commented 4 years ago

That sounds exactly right - the underlying is a treemap which isn't thread safe. My initial thoughts were that this was some kind of race condition but I was under the assumption (and still am tbh) that everything is processed in sequence.

I don't think I understand how the threading works. Assuming we have a single socket open and are receiving data in sequence (which we also want to process in sequence) what is the point of having 2 threads? Sounds like this could just lead to things being out of order.

Also, even if we do have 2 threads, aren't the observables handled in sequence and not parallel? I was under the impression events emitted by Observables would be consumed sequentially and never in parallel but I don't know much about RxJava so I'm probably wrong here.

conspyrosy commented 4 years ago

Watched the local orderbook vs the UI a little more and it still goes out of sync with just a single thread so doesn't look like thats the problem.

I also ramped up the NioEventLoop thread count to 20, wrote a couple of system outs and didn't see any overlapping of message processing. I'm not convinced increasing the thread count actually means multiple orderbook messages can be processed in parallel

So I'm stumped. I will read over the OB management code again, maybe there's a little bug somewhere that i missed on first read

earce commented 4 years ago

@badgerwithagun to @conspyrosy 's point if there are two Netty threads processing an exchange is it feasible that assuming two messages, m1 & m2 are received over the websocket sequentially, and thread t1 picks up m1 and t2 picks up m2, if t1 is slow to process m1 that t2 could push m2 into the orderbook first?

badgerwithagun commented 4 years ago

@conspyrosy, how about if you introduce random length blocking into the observer code (without using a Scheduler)?

It's possible that concurrency only manifests in practice between two observers if the observers block the NIO event loops.

Just thinking off the top of my head.