search

interledger / interledger-rs

An easy-to-use, high-performance Interledger implementation written in Rust
http://interledger.rs
Other
201 stars 70 forks source link

Reduce the amount of asserts in coinbase test to make it less flaky #701

Closed pradovic closed 3 years ago

pradovic commented 3 years ago

The coinbase test was flaky, and it started completely failing once coinbase suspended XRP. To avoid this in the future and to allow for coinbase to suspend some rates and not break our tests, I have added a simple logic to pass the test if 70% of related asserts were true.

koivunej commented 3 years ago

Re-run for the workflow after a flaky test-md.

pradovic commented 3 years ago

Re-run it several times and it failed every time. I'll try to inspect this one as well.

koivunej commented 3 years ago

I hit the re-run once more. I'd say we might want to put an -x option for bash here: https://github.com/interledger-rs/interledger-rs/blob/502365ed51fa6eb4240b87cc12865c354dc7d315/scripts/run-md.sh#L19

Also, the tcpdump idea we've chatted about for each test might work as well. I didn't implement it because the next day tests were passing. It might be best to launch from the run-md.sh. With capturing only lo traffic we should get dumps which never capture the cargo build time downloading of the dependencies, unless of course there's some system level caching config... In any case, if the dump was too large we'd just get failure to upload it so no worries. Also bz2 or lzma or zstd should compress the dumps nicely.

In #690 these failed for me the whole friday evening straight, but on the next day all passed. There's nothing obviously wrong but it could be that some cleanup action fails on the earlier cases and so the last two fail. Could be that the last two are more prone to failing just in general because of testnet stuff.

koivunej commented 3 years ago

Failed again, it was 2/4 failures when I re-ran it, now 3/4. Re-running again, looking at logs.

koivunej commented 3 years ago

Again the issue looks like before in https://github.com/interledger-rs/interledger-rs/pull/693#issuecomment-808343941, in logs/xrp-settlement/1/node-bob-settlement-engine.log:

2021-04-02T14:30:17.609Z settlement-xrp Generated new XRP testnet account: address=rnDQxEVRKFxqp3VrzgHy3bfKe4ZGDJKQGf secret=snycNRv7uQCABFKwNBZnN9mqpGbJS
2021-04-02T14:30:18.014Z settlement-core Started settlement engine server
2021-04-02T14:30:25.888Z settlement-xrp Received incoming XRP payment: xrp=0.0005 account=41248951-3a75-4368-a46a-db2dfddec394 txHash=DA33F5535B8BB26AEE1EA4A457B6EDEBC8DB1D5ED5968877DE44AFCCF8D49468
2021-04-02T14:30:25.892Z settlement-core Notifying connector to credit settlement: amountToCredit=0.0005 account=41248951-3a75-4368-a46a-db2dfddec394 settlementId=DA33F5535B8BB26AEE1EA4A457B6EDEBC8DB1D5ED5968877DE44AFCCF8D49468
2021-04-02T14:30:25.902Z settlement-core Error: Connector failed to process settlement: amountToCredit=0.0005 account=41248951-3a75-4368-a46a-db2dfddec394 settlementId=DA33F5535B8BB26AEE1EA4A457B6EDEBC8DB1D5ED5968877DE44AFCCF8D49468
2021-04-02T14:30:25.903Z settlement-core Connector credited incoming settlement: leftover=0.0005 credited=0 amountToCredit=0.0005 account=41248951-3a75-4368-a46a-db2dfddec394 settlementId=DA33F5535B8BB26AEE1EA4A457B6EDEBC8DB1D5ED5968877DE44AFCCF8D49468
[ioredis] Unhandled error event: Error: connect ECONNREFUSED 127.0.0.1:6382
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1141:16)
[ioredis] Unhandled error event: Error: connect ECONNREFUSED 127.0.0.1:6382
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1141:16)
[ioredis] Unhandled error event: Error: connect ECONNREFUSED 127.0.0.1:6382
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1141:16)
pradovic commented 3 years ago

@koivunej I was trying to reproduce it locally, unsuccessful. As you pasted, it looks like a network problem// Hmmm, but I did comment out XRP in tests, maybe it is having problem with XRP. Did not notice it before, because on the logs i was looking it looked like there was an ETH problem. Taking a look...

koivunej commented 3 years ago

@pradovic From the settlement-xrp logs it would seem that the XRP payment is received successfully, however it's unable to talk to bobs node. tcpdump would most certainly clarify this up. Also running the script in a namespace might...

koivunej commented 3 years ago

Looking at the log above more closely, the log lines are:

Notifying connector to credit settlement: https://github.com/interledgerjs/settlement-core/blob/e5c394a982d6df67585a0abf6858dabbb935a258/src/index.ts#L211

Connector failed to process settlement: https://github.com/interledgerjs/settlement-core/blob/e5c394a982d6df67585a0abf6858dabbb935a258/src/index.ts#L219

Connector credited incoming settlement: https://github.com/interledgerjs/settlement-core/blob/e5c394a982d6df67585a0abf6858dabbb935a258/src/index.ts#L239

The last one is quite confusing but it happens because there's an indecipherable response. I wonder if the response is just malformed since it doesn't seem to go to any catch handler as if it was left out of the response? The POST request: https://github.com/interledgerjs/settlement-core/blob/e5c394a982d6df67585a0abf6858dabbb935a258/src/index.ts#L199-L208

koivunej commented 3 years ago

Re-ran once more. I fear there's some end-of-week thing triggering this. Could be that it's just github load but should had lead with that tcpdump.

pradovic commented 3 years ago

Can we merge this one? I guess it is enough for coinbase flaky test + we have tcpdump for the future failures. Let's monitor and fix/change when needed in the following PRs. There should be no functional changes, so it should be ok to merge.