Closed emestee closed 6 years ago
Subscribing using
subscribe_to_orderbook([“BTC-ETH”,”BTC-NEO”, “other tickers”])
Will work fine, i.e the script will decide how many new threads to open without an issue.
Subscribing like this:
subscribe_to_orderbook([“BTC-ETH”]
subscribe_to_orderbook([”BTC-NEO”]
subscribe_to_orderbook([“other tickers”]
Would result in a bug because the code will look for an opened connection to assign the subscription and since there is none, it would open a new one. I have intentionally left it like this for the time being because most users tend to subscribe only once, using a single list. A more urgent fix is to speed up order book sync at the moment. However, as I have mentioned before I am always happy to collaborate with others who wish to make the project better, so feel free to make more PRs :)!
If you need any code interpretation feel free to ask.
P.s replying from the phone so formatting can be wrong. Hope this answers your question.
I either misunderstand the design or looking at a bug. What you're describing doesn't match what I see in the code.
Will work fine, i.e the script will decide how many new threads to open without an issue.
What it seems to actually do is to open a connection and a thread for every ticker. I'm fairly sure this is not how you want it? The initial call to subscribe_to_orderbook()
will open a connection and a thread for every ticker - _subscribe_first_run()
emits a ConnectEvent
for every ticker in the list! What is worse, these connections do not share the cf challenge, which means for every ticker you want to subscribe to, you need to undergo the puzzle and the signalr negotiation, i.e. spinup time is N * 10 sec or so - that's about 40 minutes if you want all pairs.
To mitigate it, I do something like this:
tickers = ...
first = tickers.pop()
sock.subscribe_to_orderbook([first])
sleep(10)
sock.subscribe_to_orderbook(tickers)
Which seems to do exactly what I want, i.e. all the tickers over a single thread and a single connection. However something is delaying the requests for snapshots so it still takes a long time until all tickers are fully syncronized.
Firstly, this is incorrect, it will only open a new thread+connection per ticker if you subscribe to all your tickers in separate subscribe_to_orderbook()
calls upon instantiation. As I said this is not intentional functionality but intentionally left like that for the being because there are higher priorities which I will describe below. Try putting all your tickers in a single list and then subscribe.
I am attaching a screenshot of where I have subscribed to all available tickers on Bittrex. As you can see, there are only 14 connections with 20 tickers per each, with the last one having only 8. This is also reflected in the threads: Thread1 is the queue manager, Thread2 is the order book syncer and the rest are connections. I am also attaching the log below, where you can see that full subscription was done between 2018-02-18 09:21:57 and 2018-02-18 09:23:33.
https://imgur.com/a/Fd9tM https://pastebin.com/QrSJUQGk
Obviously what you are describing is a bug but it is clearly eliminated if you put all your tickers in a single list as I have mentioned.
Secondly, what is more important to me right now is to reduce the order book synchronisation time for all pairs, which is the major slow down factor. The slow down is because when you request an order book snapshot, Bittrex doesn't provide the market name in the snapshot so you have to find what market the snapshot corresponds to. Currently this is done sequentially, so the code knows for sure what ticker snapshot it should expect.
Experimental parallel syncing committed to branch 0.0.6.3. Try it:
pip install git+https://github.com/slazarov/python-bittrex-websocket.git@0.0.6.3
Took me less than 4 minutes to sync all order books on Bittrex.
Hi,
Thank you for the detailed explanation. I did of course misunderstand your design. The good news is that I've found a solution to the problem you're describing.
In _handle_get_snapshot()
:
conn.corehub.server.invoke(method, ticker)
# hack: steal the call ID from signalr so that we can correspond snapshots to
# tickers (because of a bug in bittrex backend, snapshots have MarketName empty
call_id = conn.corehub.server._HubServer__connection._Connection__send_counter
self.ticker_call_id_map[call_id] = ticker
_is_orderbook_snapshot()
will receive an I
parameter in the relevant message, we can now look up the MarketName from ticker_call_id_map
.
I'll publish a PR shortly, need a bit of cleanup to do.
Hey, this is fixed in the experimental branch 0.0.6.3. Please take a look at it.
Hi,
Could you please explain the logic by which you're matching the snapshot that just arrived to the ticker? I'm confused about what _transfer_backorder_queue2()
does.
I also realize now that in my approach the differing connection IDs aren't accounted for, as they would repeat across several connection instances.
Side note: I've tried the new branch and it seems to work, however, something is wrong with the buy books. In is_orderbook_snapshot(), the buy books receive less than the depth. For instance for depth 500, I am seeing for BTC-DOGE 61 in buy books and 500 in sell books?
Hey, definitely. First of all _transfer_backorder_queue2
needs some renaming but that's the homework for later :). Anyways, I will explain how the code used to work and how it works now:
Pre 0.0.6.3: When you subscribed to order book syncing, the code would first invoke nounce subscription from Bittrex and then start calling for snapshots from Bittrex. The snapshot invoking was one at the time and it would not continue to the next invoke until the conditions below were satisfied:
For a small number of tickers, this is not a problem, but when you request a large number, the script would be stuck at some random ticker because it is less liquid and nounces were not coming into the stream. Note that new data is coming from the connection thread while the invokes are coming from the queue thread. Hence, other tickers were receiving their nounces but were not being synced because _handle_get_snapshot
was waiting for the illiquid ticker to get synced.
Note that each ticker has its internal queue (check _init_backorder_queue
) which is exhausted and removed when it gets synced. I have created the internal queue for two reasons. The first one is that I prefer to have some nounce history before the snapshot arrives. In this case I reduce the chances of having a snapshot nounce that is older than the message nounce. Secondly, when you are subscribing to multiple tickers, you must keep the nounces somewhere until they are synced.
Post 0.0.6.3 Snapshot invoke is now done from the connection thread. This is done under the conditions that:
This ensures that invokes are called only when there is actual data i.e nounce history, and will certainly sync. In this case we are eliminating the conditions from pre 0.0.6.3. However, how do we deal with the fact that snapshot invokes don't contain the ticker name? We have to confirm somehow that unknown snapshot Y belongs to ticker X. This is done through _is_orderbook_snapshot
-> _transfer_backorder_queue2
-> _confirm_order_book
.
_is_orderbook_snapshot
detects the id of the connection that invoked the snapshot and retrieves a list of all the tickers that belong to that connection. It then loops through the tickers by using _transfer_backorder_queue2
until it confirms the snapshot.
_transfer_backorder_queue2
acts in the same way as _transfer_backorder_queue
, however since we can't replicate queues, the only way to do accomplish that is to create a new queue and feed it with the messages from the internal queue. Essentially, we are exhausting the internal queue when we are trying to confirm the snapshot name and then we are recreating the internal queue to be used again by _transfer_backorder_queue
.
The actual name confirmation comes from _confirm_order_book
which is a shortened version of _sync_order_book
. The snapshot name is confirmed by the first ticker whose nounce difference with the snapshot is equal to zero. Note that snapshot nounces should be random and different within the connection that invoked them. This has to be tested but so far I haven't see any problems.
I hope that this make it clear, although I am most certain it would not. The code needs a cleanup :).
With respect to the side note The code applies an equal depth to both sides, so I guess the snapshot arrived like this. I will be removing the depth parameter in the next release due to users reporting bugs - (Issue #29)
Thanks for the explanation. I think that using my approach instead of relying on matching the nonces is far more foolproof, however it needs a tweak so that lookups of ticker names are done based on connection id + call ID as opposed to just call ID. I will now investigate the orderbook depth issue.
Looking forward to your solution. Best!
I'm glad to say that the new branch seems to work great, the issue with orderbook size was on my side (some whitespace ran away and took a block of code out of a for loop)
Great! Now the exception from the socket is left to be dealt with.
On Feb 19, 2018 at 5:21 pm, <Michael Stolovitzsky (mailto:notifications@github.com)> wrote:
I'm glad to say that the new branch seems to work great, the issue with orderbook size was on my side (some whitespace ran away and took a block of code out of a for loop)
— You are receiving this because you commented. Reply to this email directly, view it on GitHub (https://github.com/slazarov/python-bittrex-websocket/issues/38#issuecomment-366723926), or mute the thread (https://github.com/notifications/unsubscribe-auth/AbVUG42tksFaDV4vRdT6rP4neiG0EXcvks5tWZFhgaJpZM4SJdW2).
If you mean 503 during the websocket initialization, this is almost certainly something to do with networking. It stopped happening to me the moment I whitelisted the API IPs on my firewall so that they don't load balance between WANs.
It's coming from the websocket
library. The only viable solution I can think of is to override the methods that handle the exception. E.g in websocket._socket
we must override def recv(sock, bufsize):
. I am current researching how to do that.
Well, 503 would be an indication that cloudflare challenge we send is invalid; in my case this was due to connecting from the wrong IP.
I have noticed that towards the end of the subscription cycle the "snapshot synced" messages arrive progressively later. For example, I am looking at a running instance where 2 confirmations arrived, then half a minute later two more, and the final one never arrived. Any idea what might be causing this?
no longer relevant, closing
Sorry for piggybacking off an existing issue. I've been using v0.0.7.3 over the last week to monitor a few orderbooks and have noticed the orderbook having fallen out of sync after having come back to it a day later. I even increased the depth to 15 (suggested in another issue thread) but still had the same issue.
Curious to know if this is a known issue. Is there any workaround to this?
PS: Really appreciate your work on this slazarov!
Hi @slix88, thanks for the support.
First of all, v0.0.7.3 is archived and left for reference as I haven’t yet implemented order sync for the new version. Since then, Bittrex have improved their snapshot invoke (https://github.com/Bittrex/beta/issues/6) so half of the sync code is obsolete by now and it would be easier for you to implement it until I do it.
Secondly, it should still work but you have to remove the depth. It was a feature I had in mind in the beginning but it turned out to be a problem which caused order books to falll out of sync
When you call
subscribe_to_orderbook()
with multiple pair names, and the socket is not yet connected, every requested pair is processed in_subscribe_first_run()
, which generates a connect event (and therefore a new thread and a new connection) for every pair that requires subscription. However if you callsubscribe_to_orderbook()
with a single pair, and then wait a little and request more subscriptions, the connect events will not be generated, i.e. if you request subscriptions in bulk before the socket is connected, the library will open a connection per every subscription required. Is this intentional?