Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

New database manager #756

Closed

Conversation

ChihChengLiang
Copy link
Contributor

What was wrong?

Currently, Trinity client creates an extra process for each database consumer, this cause overheads and performance degrade.

How was it fixed?

Write a DB manager that handles all the communication to the database and a DB client that talks to the manager.

Benchmark shows the new DB manager-client has 1.5x to 2x performance comparing to the existing one.

$ python scripts/benchmark/db_managers/bench_db_client.py 
Client 1: 6412.4640859929395 get-set per second
Client 0: 6294.6387453949055 get-set per second
Client 2: 5925.177664645094 get-set per second
Client 1: 6466.534301796517 get-set per second
Client 0: 6419.005003848052 get-set per second
Client 2: 6012.5870262597755 get-set per second
Client 1: 6271.011569056251 get-set per second
Client 0: 6263.727016479054 get-set per second
Client 2: 6715.727595295333 get-set per second
$ python scripts/benchmark/db_managers/bench_old_manager.py 
Client 0: 4235.169438244839 get-set per second
Client 1: 4207.37306063274 get-set per second
Client 2: 4158.8369075974715 get-set per second
Client 0: 3686.812208983483 get-set per second
Client 1: 3679.808669849572 get-set per second
Client 2: 3590.928601338237 get-set per second
Client 0: 4002.483401664311 get-set per second
Client 1: 4020.7755111135643 get-set per second
Client 2: 4116.866458079233 get-set per second

To-Do

Cute Animal Picture

put a cute animal picture link inside the parentheses

@ChihChengLiang ChihChengLiang mentioned this pull request Jun 28, 2019
2 tasks
Copy link
Contributor Author

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to give rename suggestions and tear them apart.

tests/core/database/test_database_client.py Outdated Show resolved Hide resolved
Copy link
Member

@pipermerriam pipermerriam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's some preliminary review. If you don't mind, I might take this and re-work it a little to see whether I can find a nice middle ground. The schema.py approach seems like it may add more complexity than is strictly necessary and make the actual server and client implementations a little harder to understand. (probably not something I'll do right away).

tests/core/database/test_database_client.py Outdated Show resolved Hide resolved
trinity/db/manager/utils.py Outdated Show resolved Hide resolved
trinity/db/manager/utils.py Outdated Show resolved Hide resolved
trinity/db/manager/manager.py Show resolved Hide resolved
@ChihChengLiang
Copy link
Contributor Author

ChihChengLiang commented Jun 29, 2019

@pipermerriam Thank you for the review. Addressed the comments.

If you don't mind, I might take this and re-work it a little to see whether I can find a nice middle ground.

Feel free to do so.

@pipermerriam pipermerriam mentioned this pull request Jul 30, 2019
2 tasks
@cburgdorf
Copy link
Contributor

Closing as this has landed with #859

@cburgdorf cburgdorf closed this Aug 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants