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

Add concurrency for reward calculation to scale out #1291

Merged
merged 20 commits into from
May 27, 2022

Conversation

prasannavl
Copy link
Member

@prasannavl prasannavl commented May 25, 2022

/kind fix

  • Scales out the owner rewards calculation in a naive way for now to speed up token splits.

Example of mainnet simulated splits:

  • With a 16 core parallel run, this brings down the time from 4.2 minutes to 45 secs.

Without:

2022-05-25T08:27:21Z Preparing for token split (id=24, mul=2, n=1/1, height: 1912900)
2022-05-25T08:27:21Z Pools to migrate for token 24: (count: 1, ids: 42)
2022-05-25T08:27:21Z new proof-of-stake block found hash: 93603c6cb5d05d7201f047cd62b9208d613ab72f05367082bcabd4a553a2b2c6
2022-05-25T08:27:21Z Preparing for token split (id=24, mul=2, n=1/1, height: 1912900)
2022-05-25T08:27:21Z Pools to migrate for token 24: (count: 1, ids: 42)
2022-05-25T08:27:21Z Token split in progress.. (id: 24, mul: 2, height: 1912900)
2022-05-25T08:27:21Z Token split info: (symbol: ARKK, id: 24 -> 83)
2022-05-25T08:27:21Z Pool migration in progress.. (token 24 -> 83, height: 1912900)
2022-05-25T08:27:21Z Pool migration: Old pair (id: 42, token a: 24, b: 15, reserve a: 4800698937906, b: 204262939800401, liquidity: 30914238651261)
2022-05-25T08:27:21Z Pool migration: Migrating 681 balances.. 
2022-05-25T08:31:38Z DEBUG:: Pool migration: migration time 256395ms 
2022-05-25T08:31:38Z Pool migration: New pair (id: 84, token a: 83, b: 15, reserve a: 9601397875812, b: 204262939800401, liquidity: 44285547938233)
2022-05-25T08:31:38Z Pool migration complete: (42 -> 84, height: 1912900, time: 256635ms)
2022-05-25T08:31:38Z Token split info: Rebalance (id: 24, symbol: ARKK, add: 864, sub: 864, total: 10122216198168)
2022-05-25T08:31:38Z Vaults rebalance in progress.. (token 24 -> 83, height: 1912900)
2022-05-25T08:31:38Z Vaults rebalance completed: (token 24 -> 83, height: 1912900, time: 113ms)
2022-05-25T08:31:38Z Token split completed: (id: 24, mul: 2, time: 256962ms)
2022-05-25T08:31:38Z Token split block validation time: 256992.47ms

In parallel:

2022-05-25T14:33:06Z Preparing for token split (id=24, mul=2, n=1/1, height: 1912890)
2022-05-25T14:33:06Z Pools to migrate for token 24: (count: 1, ids: 42)
2022-05-25T14:33:06Z new proof-of-stake block found hash: 616afb4c98c6728a374636df995f3f0a90f814b93b6f62bad01c88199123af82
2022-05-25T14:33:06Z Preparing for token split (id=24, mul=2, n=1/1, height: 1912890)
2022-05-25T14:33:07Z Pools to migrate for token 24: (count: 1, ids: 42)
2022-05-25T14:33:07Z Token split in progress.. (id: 24, mul: 2, height: 1912890)
2022-05-25T14:33:07Z Token split info: (symbol: ARKK, id: 24 -> 83)
2022-05-25T14:33:07Z Pool migration in progress.. (token 24 -> 83, height: 1912890)
2022-05-25T14:33:07Z Pool migration: Old pair (id: 42, token a: 24, b: 15, reserve a: 4800698937906, b: 204262939800401, liquidity: 30914238651261)
2022-05-25T14:33:07Z Pool migration: Migrating 681/114198 balances..
2022-05-25T14:33:07Z Pool migration: concurrency: 16
2022-05-25T14:33:52Z Pool migration: rewards time: 44847.15ms
2022-05-25T14:33:52Z Pool migration: New pair (id: 84, token a: 83, b: 15, reserve a: 9601397875812, b: 204262939800401, liquidity: 44285547938233)
2022-05-25T14:33:52Z Pool migration complete: (42 -> 84, height: 1912890, time: 45106ms)
2022-05-25T14:33:52Z Token split info: Rebalance (id: 24, symbol: ARKK, add: 864, sub: 864, total: 10122216198168)
2022-05-25T14:33:52Z Vaults rebalance in progress.. (token 24 -> 83, height: 1912890)
2022-05-25T14:33:52Z Vaults rebalance completed: (token 24 -> 83, height: 1912890, time: 105ms)
2022-05-25T14:33:52Z Token split completed: (id: 24, mul: 2, time: 45457ms)
2022-05-25T14:33:52Z Token split block validation time: 45491.06ms

@prasannavl
Copy link
Member Author

On hold for now. There are atleast 3 - GetBalancesHeight , GetShare and GetBalance + more in CalculatePoolRewards that are isolated Read calls. While leveldb operates on a single point in time snapshot for iterators, which will be consistent the isolated Read calls can see data across different points, since all of the temporary views still will hit the same db.

There needs to be additional safety.

@prasannavl
Copy link
Member Author

prasannavl commented May 25, 2022

Alternatively, can go the other way, and merge changeset. That should be free of levedb writes - the reason it however appears to work so far and adds up in tests may be because all of the writes are per owner only, and as such the writes don't clobber the other reads to end up in partial states. It's technically unsynchronized, but in this very specific context, it's never runs into trouble.

Perhaps, the closest philosophical analogy is to having global array, reading them from multiple threads, but making promises that we'll only ever write to a particular range per thread that aren't seen by other threads, and there by free of races. Works, but unsafe and fragile.

May be possible to get away with it for the moment as a short term solution if these assumptions can be verified further, though this cannot be a permanent solution as these assumptions can get broken anytime.

@prasannavl
Copy link
Member Author

prasannavl commented May 26, 2022

Newer strategy, given some of the assumptions above still holds true - uses 2 thread pools:

  • 1 pool that scales out linearly to near the hardware cores for owner rewards recalc.
  • Another single threaded pool that receives IO requests (all the flushes) and serializes, one by one.

This is now verifiably faster (~24 secs) than an synchronized in-mem changeset merge, as well as manually synchronized flushes. Also note that boost asio internally synchronises the pool dispatch and posts, so eliminates the need for external locks.

Sample:

2022-05-26T04:23:48Z Preparing for token split (id=24, mul=2, n=1/1, height: 1912920)
2022-05-26T04:23:48Z Pools to migrate for token 24: (count: 1, ids: 42)
2022-05-26T04:23:48Z Token split in progress.. (id: 24, mul: 2, height: 1912920)
2022-05-26T04:23:48Z Token split info: (symbol: ARKK, id: 24 -> 83)
2022-05-26T04:23:48Z Pool migration in progress.. (token 24 -> 83, height: 1912920)
2022-05-26T04:23:48Z Pool migration: Old pair (id: 42, token a: 24, b: 15, reserve a: 4800698937906, b: 204262939800401, liquidity: 30914238651261)
2022-05-26T04:23:48Z Pool migration: Migrating balances (count: 681, total: 114198, concurrency: 16)..
2022-05-26T04:24:12Z Pool migration: rewards recalc time: 23574.54ms
2022-05-26T04:24:12Z Pool migration: New pair (id: 84, token a: 83, b: 15, reserve a: 9601397875812, b: 204262939800401, liquidity: 44285547938233)
2022-05-26T04:24:12Z Pool migration complete: (42 -> 84, height: 1912920, time: 23830ms)
2022-05-26T04:24:12Z Token split info: rebalance (id: 24, symbol: ARKK, add-accounts: 799, sub-accounts: 799, add: 10120603809306, scanned: 114198)
2022-05-26T04:24:12Z Vaults rebalance in progress.. (token 24 -> 83, height: 1912920)
2022-05-26T04:24:12Z Vaults rebalance completed: (token 24 -> 83, height: 1912920, time: 108ms)
2022-05-26T04:24:12Z Token split completed: (id: 24, mul: 2, time: 24206ms)
2022-05-26T04:24:12Z Token split block validation time: 24217.63ms

@prasannavl prasannavl changed the title Add concurrency for reward calculation to scale out [WIP] Add concurrency for reward calculation to scale out May 26, 2022
@prasannavl prasannavl changed the title [WIP] Add concurrency for reward calculation to scale out Add concurrency for reward calculation to scale out May 26, 2022
@prasannavl prasannavl force-pushed the fix/concurrent_rewards_calc branch from dcadd9f to 7f6e5ea Compare May 27, 2022 04:50
@prasannavl prasannavl force-pushed the fix/concurrent_rewards_calc branch from 7f6e5ea to a95ca0e Compare May 27, 2022 04:51
@prasannavl prasannavl merged commit 7dce4c2 into master May 27, 2022
@prasannavl prasannavl deleted the fix/concurrent_rewards_calc branch May 27, 2022 05:19
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

Successfully merging this pull request may close these issues.

2 participants