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

Implementation of the context timing #542

Merged

Conversation

sebastiencs
Copy link
Contributor

This exposes the context stats on the RPC endpoint GET /stats/context.

Data is stored in a sqlite database from the protocol runner process, where statistics are computed.
When the RPC endpoint is called, it reads from that sqlite file and formats it according to the example JSON I got.
Note that the endpoint expose only tezedge stats, even though irmin ones are in the db.

  • I'm not sure what should be the path of the database file ?
  • There is no tests yet on the RPC endpoint, should I include an already filled sqlite database in the repository ? So that the tests would just read from it

Copy link
Contributor

@tizoc tizoc left a comment

Choose a reason for hiding this comment

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

@sebastiencs overall it looks good, I left some comments.

Also, I think it would be better to use the rusqlite crate, seems to have a more convenient API, more users, support for transactions, etc. Some of the things may not be needed now but if we extend this to be a replacement for the current context actions storage, those are going to be come useful (because those features will help with optimizing things more).

tezos/new_context/src/timings.rs Outdated Show resolved Hide resolved
tezos/new_context/src/timings.rs Outdated Show resolved Hide resolved
rpc/src/services/context.rs Outdated Show resolved Hide resolved
tezos/new_context/src/timings.rs Outdated Show resolved Hide resolved
@tizoc
Copy link
Contributor

tizoc commented May 4, 2021

Oh and about your questions

I'm not sure what should be the path of the database file ?

Good question. I am not sure yet, add some default and we can figure it out later.

There is no tests yet on the RPC endpoint, should I include an already filled sqlite database in the repository ? So that the tests would just read from it

For the test you can initialize the Sqlite store in-memory, and insert whatever values you want to it, no need for an on-disk file.

@tizoc
Copy link
Contributor

tizoc commented May 4, 2021

In-memory SQlite database: https://sqlite.org/inmemorydb.html

@tizoc tizoc force-pushed the protocol-runner-storage branch from cf289bf to 2de786c Compare May 4, 2021 19:24
@sebastiencs sebastiencs force-pushed the protocol-runner-storage-opti branch from 0b982c6 to c66a99d Compare May 5, 2021 10:53
rpc/src/services/context.rs Outdated Show resolved Hide resolved
rpc/src/services/context.rs Outdated Show resolved Hide resolved
@tizoc tizoc marked this pull request as ready for review May 6, 2021 13:49
@tizoc tizoc merged commit 0077d79 into tezedge:protocol-runner-storage May 6, 2021
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.

None yet

2 participants