Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connection to multiple market - PANIC #144

Open
dklodner opened this issue Jan 10, 2019 · 17 comments
Open

Connection to multiple market - PANIC #144

dklodner opened this issue Jan 10, 2019 · 17 comments

Comments

@dklodner
Copy link

Hi,
I am trying to connect to multiple markets but the program keeps crashing in UpdateWidth function (orderbook.go).

Here's the code to reproduce this issue. It takes a minute or so to get the error message.
https://paste.ubuntu.com/p/yrMhhDBRsn/

What am I doing wrong? Thanks for your help...

David

@JacobPlaster
Copy link
Contributor

JacobPlaster commented Jan 14, 2019

Hi @dklodner,

Thanks for reporting this, your code is perfectly fince. The code that calculates the checksum of the orderbook had a small bug which only caused a problem when the orderbooks got thin. I have just merged a fix for this which you can see here #145.

Please could you make sure you have the latest commit on master an try again. Let me know if you have any more problems with it.

@dklodner
Copy link
Author

dklodner commented Jan 14, 2019

Hii @JacobPlaster, it's still an issue ...

`
2019/01/14 14:44:53 Orderbook 'tEOSUSD' checksum verification successful.
2019/01/14 14:44:53 [DEBUG]: {"event":"unsubscribed","status":"OK","chanId":74264}
2019/01/14 14:44:53 [DEBUG]: [74264,"cs",-2129011831]
2019/01/14 14:44:53 MSG RECV: &websocket.UnsubscribeEvent{Status:"OK", ChanID:74264}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x626989]

goroutine 103 [running]:
github.com/bitfinexcom/bitfinex-api-go/v2/websocket.(*Client).handleChecksumChannel(0xc420158180, 0x12218, 0xffffffff8119db89, 0x0, 0x0)

`

@dklodner
Copy link
Author

dklodner commented Jan 14, 2019

@JacobPlaster, another one with concurrent map read and write error:
https://pastebin.ubuntu.com/p/yWV7fTHkP6/

@dklodner
Copy link
Author

Sorry to keep spamming, but it's throwing different error messages:

2019/01/14 17:45:22 [DEBUG]: [147316,[32.78,0,1]]
panic: runtime error: index out of range

goroutine 169 [running]:
github.com/bitfinexcom/bitfinex-api-go/v2/websocket.(*Orderbook).Checksum(0xc4208e9280, 0xc4201b8090)
.../Golang/src/github.com/bitfinexcom/bitfinex-api-go/v2/websocket/orderbook.go:75 +0x5c6

@JacobPlaster
Copy link
Contributor

JacobPlaster commented Jan 15, 2019

Ok thanks @dklodner thats no problem, thanks for pasting them here because I was struggling to reproduce this. Im going to look into this again for you

@dklodner
Copy link
Author

@JacobPlaster
Copy link
Contributor

JacobPlaster commented Jan 15, 2019

@dklodner I've just merged a PR which hopefully fixes all of the issues you have pointed out above (see PR #146). This adds some extra error catching and sync locking.

I've been running your script for 3 hours on my changes and havent seen any crash's so please let me know if you still get any errors. Also, if you do see errors in the meantime try disabling the crc32 orderbook verification for a short term fix i.e p.ManageOrderbook = false

@dklodner
Copy link
Author

Thank you @JacobPlaster for your work on this issue. There's a new error with concurrent map write. The program seems to try to connect multiple times to the same ticker...

https://pastebin.ubuntu.com/p/HVjgck8mxt/

@JacobPlaster
Copy link
Contributor

JacobPlaster commented Jan 15, 2019

You're welcome. I'll take a look into this one tomorrow, thanks for the pastebin.

@JacobPlaster
Copy link
Contributor

OK, it seems like the are a few areas that need a sync map lock. Ill get that merged in soon. @dklodner Is everything working correctly for you with manageorder book disabled?

@dklodner
Copy link
Author

It seems stable with manageorderbook disabled.

@dklodner
Copy link
Author

dklodner commented Feb 9, 2019

Hi @JacobPlaster. Would you please take a look at this issue? Is there any way how to handle 429 status code? Also program panics after all reconnection attempts finish.

https://paste.ubuntu.com/p/49w26kFYmq/

@JacobPlaster
Copy link
Contributor

Hi @dklodner sorry for the late reply. Ill check that out for you

@JacobPlaster
Copy link
Contributor

@dklodner again sorry for the late reply. Did you manage to sort this out? It seems like the above error that you posted was caused by our API gateway rate limits

@dklodner
Copy link
Author

API rate limiting can't crash the program, don't you think? It looks like the library isn't handling 429 rate limiting error messages (increasing reconnection timeouts?) and it panics once the reconnection process ends after predefined number of tries.

@JacobPlaster
Copy link
Contributor

JacobPlaster commented Mar 22, 2019

@dklodner yes I agree, but It doesnt seem like its the rate limiting bad hanshake is causing the crash here. The library attempts the reconnect which is to be expected but does not panic until it exhausts all of its attempts and then finally proceeds to kill the process. I think this is more of a problem with the lib not shutting down gracefully. I wonder if you would still get this problem if you increased the reconnection timeouts/attempts so that the lib re-establishes the connection using something like this:

p := websocket.NewDefaultParameters()
// Enable orderbook checksum verification
p.ReconnectAttempts = 100
p.ReconnectInterval = time.Second * 60
c := websocket.NewWithParams(p)

err := c.Connect()
if err != nil {
    log.Fatal("Error connecting to web socket : ", err)
}

The bitfinex API limits can vary in a range of 10 to 90 requests per minute so setting the interval to be a minute should reset the limits. In the meantime though ill definitely look into the shutdown logic of the lib because even though the library is shutting down it should not be causing a panic

@dklodner
Copy link
Author

That's what I ended up doing. It would be great if you were able to find a permanent solution. Thanks for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants