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

pi: Cache proposal statuses. #1586

Merged
merged 45 commits into from Dec 3, 2021
Merged

pi: Cache proposal statuses. #1586

merged 45 commits into from Dec 3, 2021

Conversation

amass01
Copy link
Member

@amass01 amass01 commented Nov 24, 2021

Background:

This diff aims to improve a performance issue where the pi summary
command for proposals in the "Approved" politeiagui tab required two
different calls to be made to the tstore backend in order to determine
their status. The first call retrieves record metadata and the second
call retrieves billing status metadata. Each of these calls result in
the tlog tree being retrieved. A ticketvote vote summary request is
also needed to get the current vote status of the proposals.

Implementation:

This commit adds an in-memory cache which is used to cache proposal data
required to determine the proposal status at runtime such as record
metadata, vote metadata, the vote status and the proposal billing status
changes. The cache is necessary to improve the performance of
determining a status of a proposal at runtime by reducing the number of
expensive backend calls that result in the tlog tree be retrieved, which
gets very expensive when a tree contains tens of thousands of ticket
vote leaves. This is helpful when the cached data is not expected to
change, which means that once we store the data in cache we don't need
to fetch it again. The cache entries are lazy loaded and it's capacity
is limited to 1000 entries.

Benchmark:

We have tested this with 5 proposals that have an active proposal status
and with ~1000 tlog leaves each. The pi summaries call took ~675ms
without the proposal status cache and ~200ms with the proposal status
cache. A ~70% improvement.


Part of #1582.

@amass01 amass01 changed the title [wip] pi: cache proposal statuses. pi: cache proposal statuses. Nov 28, 2021
@amass01 amass01 marked this pull request as ready for review November 28, 2021 00:41
Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

Needs work.

Capitalize the cache in the commit message title please.

politeiad/backendv2/tstorebe/plugins/pi/pi.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/pi.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/cmds.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/pi.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/pi.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/cmds.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/cmds.go Outdated Show resolved Hide resolved
@amass01 amass01 changed the title pi: cache proposal statuses. pi: Cache proposal statuses. Nov 29, 2021
politeiad/backendv2/tstorebe/plugins/pi/cache.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/cmds.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/cache.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/cmds_test.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/cmds_test.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/proposalstatus.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/proposalstatus.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/proposalstatus.go Outdated Show resolved Hide resolved
politeiad/backendv2/tstorebe/plugins/pi/statusescache.go Outdated Show resolved Hide resolved
Copy link
Member

@lukebp lukebp left a comment

Choose a reason for hiding this comment

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

tACK

@lukebp lukebp merged commit 7f42cfe into decred:master Dec 3, 2021
@amass01 amass01 deleted the propstatuscache branch December 3, 2021 21:17
@lukebp lukebp added 91cfcc8 performance-impovement pi-not-deployed This item has been merged into master, but has not been deployed to production yet. labels Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance-impovement pi-not-deployed This item has been merged into master, but has not been deployed to production yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants