Skip to content
This repository has been archived by the owner on Jun 8, 2020. It is now read-only.

Gemini Inconsistent OrderBook #105

Closed
traviscollins opened this issue Feb 21, 2018 · 6 comments
Closed

Gemini Inconsistent OrderBook #105

traviscollins opened this issue Feb 21, 2018 · 6 comments

Comments

@traviscollins
Copy link
Contributor

I have a long running Websocket running for Gemini subscribed to the BTC/USD and ETH/USD channels. Both channels are apparently floating to inconsistency in the local order book with the Gemini side orderbook.

Over long periods of time, I've seen the highest price BID order book entry become a stale entry that remains for long periods of time. Restarting the JVM (causing a reconnect to the web socket) immediately fixes the issue.

Has anyone else observed this behavior?

@traviscollins traviscollins changed the title Gemini inconsistent OrderBook Gemini Inconsistent OrderBook Feb 21, 2018
@traviscollins
Copy link
Contributor Author

traviscollins commented Feb 25, 2018

I believe I've nailed down this issue. Caused by the use of BigDecimal (a non-thread-safe class) as the key in the GeminiOrderBook price level map for asks/bids internally. The same BigDecimal instance is used as the value in the level - and can therefore be accessed by outside classes. This allows outside methods to access the BigDecimal object, which appears to cause possibly differing hashCodes (when two threads are accessing the same BigDecimal at once).

See GeminiOrderBook.java line 44.

Will test for a few days to ensure the fix is correct, and then submit a PR.

traviscollins pushed a commit to traviscollins/xchange-stream that referenced this issue Feb 25, 2018
@Dclusin-og
Copy link

Isn't the order book only ever supposed to be touched on the event loop thread? Where's the other thread coming from? I'm curious as I'm building against this.

@traviscollins
Copy link
Contributor Author

Multi-threaded application that depends on this library uses the orderbook in another thread.

@Dclusin-og
Copy link

Dclusin-og commented Feb 27, 2018

You would probably want to make a deep copy of the order book each time you hand it off to another thread. The whole architecture of this library is designed with a single thread use case in mind, hence the data structures it uses are not thread safe. You've only solved one minor issue, lots more will pop up since you're using data structures that are not intended to be shared amongst threads without external syncrhonization.

One example: In the code for OrderBook it uses an ArrayList to store the limit orders for each side. If you use this across multiple threads and then try to iterate over it in your thread and the library comes and updates the rate book, your thread will receive a ConcurrentModificationException (per the ArrayList iterator java docs).

Copying the book will solve concurrency issues at the cost of application performance.

@traviscollins
Copy link
Contributor Author

traviscollins commented Feb 27, 2018 via email

dozd pushed a commit that referenced this issue Feb 27, 2018
dozd pushed a commit that referenced this issue Feb 27, 2018
* Fix for issue #105

* Fix for issue #111.
@dozd
Copy link
Member

dozd commented Feb 27, 2018

Thanks for the fix!

@dozd dozd closed this as completed Feb 27, 2018
dozd pushed a commit that referenced this issue Mar 6, 2018
* Fix for issue #105

* Fix for gemini orderbook inconsistency.

* Fix for gemini orderbook inconsistency unknown message types.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants