Race can occur when subscribing to markets on an open socket about betfair HOT 10 CLOSED

You able to detail the steps to replicate?

from betfair.

I've misplaced the stack trace too, my apologies, but I could replicate it given more time.
I don't have a test case right now (give me more time, if you still need it), but I'll describe how I replicated it.
I got this on an inplay market using the live key, following several matches (ie lots of incoming prices).

First I setup a stream and subscribed to some markets as per normal. Then I added a scheduler to call subscribe_to_markets periodically, the need to re-subscribe to markets is to keep under the subscription limit that betfair have. I did not stop the socket when the scheduler called subscribe_to_markets.
The key thing that caused the error was being subscribed to many liquid markets with incoming data, then subscribing to them again without stopping/closing the socket first.
I also suffer some internet lag occasionally which may have caused some incoming mcm to arrive after the new MarketStream/cache was created.

from betfair.

So subscribing to markets isn't additive, when you run subscribe to markets it replaces the market stream in the listener. I assume that inbetween this happening the socket returns an update (mcm) from the previous subscription but the market stream / cache has been reset causing a KeyError.

However you shouldn't get a KeyError as I added error handling for this scenario on line 101 in stream.py, intrigued as to where it is raising an error.

from betfair.

Yes that's exactly what I'm struggling to say :)
I get the logging message on line 101 but then immediately get the KeyError on line 104.
I'll get you a stacktrace that I can publish in the next couple of days.

from betfair.

Forgot to mention that I am running the socket with async=True too which might make a difference.

from betfair.

Ah, I think that inbetween 101 and 104 the cache is getting removed or maybe the entire Stream object is being garbage collected due to it being overwritten when creating a new stream.

When creating a new subscription need a way to disregard any updates from the old subscription, as you mentioned unique id would work but need to maybe take it away as a variable and set it to an incrementing number that BetfairStream controls.

from betfair.

In my use case it would be OK to keep accepting updates from the old subscription - most of the subscription is the same. Before I filed this bug I experimented with calling register_stream() only if a market_stream didn't already exist and it didn't suffer from this race bug, but I wanted to be stricter about validating that it was changing the subscription of the same Stream/MarketBook objects so I rolled that change back.

from betfair.

At the moment listener has OrderStream and MarketStream, maybe it can be changed to a dictionary with the unique Id as they Key. This would allow the listener to handle multiple streams and prevent the above.

I know there was a reason for not using a dictionary I just can't remember what it was...

from betfair.

Speed?
I have an idea for a fix that will not re-create the MarketStream if it already exists. I'll put it up later, for people to comment upon.

from betfair.

@liampauling , other contributors, I'm having some trouble fixing the unit test, but in the meantime can I get your thoughts on this commit?
oughtotrade@daa88eb

It avoids the race by only creating the new stream when registering to a stream for the first time. The unique_id is updated when re-subscribing to a stream. As the id is not a very strict id (my impression from the BetFair documentation) I didn't feel the need to keep a version of the cache/MarketBook per id. Instead it can just keep re-using the current cache.
Upon re-subscribing to a stream, bfl will get a SUB_IMG from the stream but _process can handle this safely.
Thanks.

from betfair.