Performance tests

[ayrat555]

Subtask of #59 and #141

LoadTest.Runner.WatcherInfoAccountApi

Current behaviour

It creates and funds a single account on the childchain and then measures API responses multiple times for:

1. /account.get_balance
2. /account.get_utxos
3. /account.get_transactions
4. /transaction.create - the funded account sends the value back to the faucet account

No assertions are made during this test

Suggested behaviour

Split this test into two separate tests:

1. Run multiple iterations of:

• create account
• send some amount to the created account on the childchain
• validate amount with /account.get_balance
• validate that the account has the required utxos with /account.get_utxos

2. Run multiple iterations of

• create and fund account
• send some amount from created account to another account with /transaction.create
• validate created transaction with /transaction.create

LoadTest.Runner.UtxosLoad

Current behaviour

It has two scenarios:

1. Funds the account and creates multiple utxos
2. Sends utxos to another account

Suggested behaviour

It can be separated into two tests:

1. The first test will create multiple utxos for different accounts validating it with /account.get_utxos
2. The second test will spend utxos validating balances.

As you can see in this case I'm suggesting only to add assertions.

LoadTest.Runner.StandardExits

Current behaviour

• Create many utxos
• Start exits for the created utxos

Suggested behaviour

• Create many utxos validating that utxos are created
• Start exits validating balances on the rootchain

LoadTest.Runner.Smoke

Current behaviour

• checks status api endpoints for childchain, watcher_info and watcher

Expected behaviour

I don't think it has to be ported (I don't think it's needed)

LoadTest.Runner.ChildChainTransactions

Current behaviour

• funds the account
• spends utxos

Expected behaviour

It seems this test is already part of LoadTest.Runner.UtxosLoad.

For now, I won't delete existing tests, I will just create new ones that can be run from the command line using mix run. You can check an example of such test in omgnetwork/elixir-omg#1745

[InoMurko]

First of all, the work being done here should be exclusively focusing in providing performance tests for the Childchain (2). Priority wise. We can shift our focus to Watcher APIs when it's needed (and this is done).

Now I'll focus exclusively on those two paragraphs that test the childchain.
LoadTest.Runner.StandardExits - what is the objective here? @boolafish are there any mass exit numbers we could validate here? Exits are being "detected and processed" by both childchain and watchers (independently). So it makes sense to test them separately too.
LoadTest.Runner.Smoke - the childchain does not have a "status" endpoint.
LoadTest.Runner.ChildChainTransactions here's my concern: if we're testing LoadTest.Runner.UtxosLoad -> Watcher -> Childchain, we're gonna get skewed results because Watcher will interfere. How do we mitigate that?

[boolafish]

I will put my feedback of each section:

LoadTest.Runner.WatcherInfoAccountApi

Suggested behaviour

Split this test into two separate tests:

This test was created to target to test out performance the watcher-info account related APIs. But...as known, no assertion was done at the time. Personally I am interested with assertion with API latency (server side), and error rate (probably both client and server?) for all APIs that is targeting to test.

The test sends transaction to the account itself in able to increase the load of DB to same account. So we can probably see performance deviation on having more and more data bind to same account.

As your suggested behavior mentioned some deposit and transfer test, isn't that covered by your current WIP PR?

LoadTest.Runner.UtxosLoad

Suggested behaviour

It can be separated into two tests:

I am confused actually. The reason of 2 parts I think is to fund the account so the later part can iterate. (If I recall it correctly)

As you can see in this case I'm suggesting only to add assertions.

I would like to add assertion on our childchain submit tx API. This is our core function. Same as above, server latency metric and error rate are something that is definitely worth asserting.

LoadTest.Runner.StandardExits

Suggested behaviour

Unlike other ones, we probably are not too interested in API performance for this one. This test was created to ensure our system can handle some loads on concurrent exit happening (we had an incident previously when some exits happens and infura has some issue, it crashes).

So probably what you're planning to do is good enough? Or we can add fetch some generic system monitoring metric to ensure nothing happens.

LoadTest.Runner.Smoke

Expected behaviour

I don't think it has to be ported (I don't think it's needed)

I was just about to agree with you. But then I see your PR implementation that would be really protective and perf run and not fail fast.

The smoke test is ... not to test but to ensure the connection is set correctly on perf configs so we don't waste time run the long running ones. But I don't mind remove this and see how much it is an issue.

LoadTest.Runner.ChildChainTransactions

Expected behaviour

It seems this test is already part of LoadTest.Runner.UtxosLoad.

Actually I have forgot the reasoning of having two similar ones. They are slightly different on how UTXOs are used but what was the main goal?? cc @kevsul do you still remember.

[boolafish]

LoadTest.Runner.StandardExits - what is the objective here? @boolafish are there any mass exit numbers we could validate here? Exits are being "detected and processed" by both childchain and watchers (independently). So it makes sense to test them separately too.

This was introduced as there was incident that our system failing when many exit occurs. We actually don't need too much as last incident was 14 exits 😅 so I recall the conclusion of part of the release acceptance something 15 might be good enough lol

LoadTest.Runner.ChildChainTransactions here's my concern: if we're testing LoadTest.Runner.UtxosLoad -> Watcher -> Childchain, we're gonna get skewed results because Watcher will interfere. How do we mitigate that?

This test goes directly to childchain, no watcher in the middle I believe. It is chaining the utxos.

[kevsul]

Yeah, the performance tests all have different objectives:

1. LoadTest.Runner.ChildChainTransactions
   Tests if the child chain can accept a large amount of concurrent transactions. The theoretical limit of the network is 64k transactions per block, which is around 4200 tps. In practice we hit other limits such as DB timeouts earlier.
   Ideally we should know what tps the child chain can handle indefinitely, and what peak number above that we can handle for how long e.g. we can do 1000 tps forever, but we can only handle 4000 tps for 1 hour.

2. LoadTest.Runner.UtxosLoad
   Tests how the system performs when there are a lot of utxos present. The issue here is that the child chain needs to validate every new transaction to check that it's not double spending, so as the utxo set increases this validation will take longer.
   The test first creates a large number of utxos and then submits new transactions so that the time taken to accept them can be measured.
   Metrics to extract from this would be something like:

Total Utxos	submitTransaction response time
10K	100 ms
100K	2000 ms
1M	20 s
10M	300 s

3. LoadTest.Runner.StandardExits
   As Andy said, this is to measure how many exits the system can handle. Inspired by this issue https://github.com/omgnetwork/private-issues/issues/58 where the watcher crashed when resyncing and processing a handful (only 14!) exits at once. That issue was in the watcher, but it would be good to test if the child chain has a problem with many exits as well.

In general the purpose of these tests is to test how the system performs under load - "the system" includes both the child chain and the watcher, but I agree that we should focus on the child chain first.
I'm not sure if testing for correctness is necessary here (e.g. asserting balances, etc.), that should probably be already tested elsewhere. What's more interesting is the response time of the child chain. As I described above, we were using calls to submitTransaction as a metric for that, but there may be a better way.

[ayrat555]

I'm not sure if testing for correctness is necessary here (e.g. asserting balances, etc.), that should probably be already tested elsewhere. What's more interesting is the response time of the child chain. As I described above, we were using calls to submitTransaction as a metric for that, but there may be a better way.

I think there are module tests and integration tests (specs) which test some of these features. But isn't the purpose of #59 to test the correctness under load?

Based on your comments I think I can implement a test based on LoadTest.Runner.UtxosLoad which will:

1. create utxo
2. check balance creating
3. spend utxo
4. check balance after spending

Also, as @boolafish suggested in #149 (comment) I'll add the required metrics to deposits and utxo tests.

@InoMurko @boolafish are you ok with this?

[kevsul]

I think there are module tests and integration tests (specs) which test some of these features. But isn't the purpose of #59 to test the correctness under load?

Yeah, that's true.
In the past I found that it was difficult to get high tps if the client code was doing anything else other than just firing out requests, but I guess the proper solution for that is to run more clients in parallel.

[boolafish]

Probably also check the feedback (slack) from infra team to see if there is other metrics to add to the tests: https://omgnetworkhq.slack.com/archives/CMX790Q5V/p1603252908142800?thread_ts=1603249243.137000&channel=CMX790Q5V&message_ts=1603252908.142800

[InoMurko]

I think there are module tests and integration tests (specs) which test some of these features. But isn't the purpose of #59 to test the correctness under load?

Yeah, that's true.
In the past I found that it was difficult to get high tps if the client code was doing anything else other than just firing out requests, but I guess the proper solution for that is to run more clients in parallel.

Absolutelly. Stuffing this thing into containers and run them in parallel should be one of our goals, but it is also important that once an instance finishes, it asserts that what it's been doing resulted in the correct ending balance. We need to be sure that our accounting works as designed even when our stack is under high load.

I would also insist, that when the tests are finished, we would be able to (for the purpose of the test - for now) export a report from the childchain, with all needed columns that assert that our accounting finished with ✔️.

[ayrat555]

Related prs in elixir-omg:
omgnetwork/elixir-omg#1745
omgnetwork/elixir-omg#1756
omgnetwork/elixir-omg#1753
omgnetwork/elixir-omg#1757

[InoMurko]

Done, thanks @ayrat555