-
Notifications
You must be signed in to change notification settings - Fork 677
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
Chore/cache timestamp calc #5494
Chore/cache timestamp calc #5494
Conversation
Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
…previous) into tenure_blocks Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM! You have some compilation issues in nakamoto_integrations
…acks-network/stacks-core into chore/cache-timestamp-calc
…ecution cost of block validation Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
…acks-network/stacks-core into chore/cache-timestamp-calc
…ock info extracted Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could be wrong about the comment of "do we need to query by state" - but that's really all I saw. Nice work!
You are right! I missed it when converting to use the same table! |
…et_tenure_times Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending CI! Please also test it out together with #5471 and see if those tests still work.
DROP TABLE blocks; | ||
|
||
ALTER TABLE temp_blocks RENAME TO blocks;"#; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏 👏 👏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a lot better! Before approving, I just had one question about the use of user-define sqlite functions.
Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving pending CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Very clever migration scheme - I like it.
Signed-off-by: Jacinta Ferrant <jacinta@trustmachines.co>
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
I originally tried to create a db that just tracked a cumulative tenure processing time that would reset each time a new tenure change payload arrived, but these had a lot of edge cases due to out of order blocks. Hank suggested keeping a much smaller db copy of blocks to determine the tenure processing time instead which was much cleaner. I also implemented a cleanup for it since technically this is redundant/heuristic info and don't think we should have to keep it around indefinitely like we do other signer data.