feat(fortuna): refactor explorer endpoint & history query methods #2754
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
cprussin
force-pushed
the
cprussin/refactor-fortuna-explorer-endpoint
branch
from
7801106 to
c4e924d
Compare
| @@ -2,3 +2,5 @@ | |||
| *config.yaml | |||
| *secret* | |||
| *private-key* | |||
| .envrc | |||
| fortuna.db | |||
There was a problem hiding this comment.
@m30m are these safe to gitignore? I noticed they weren't here before; I assume it's just an oversight but was there a good reason?
cprussin
force-pushed
the
cprussin/refactor-fortuna-explorer-endpoint
branch
from
c4e924d to
6ce6c23
Compare
cprussin
force-pushed
the
cprussin/refactor-fortuna-explorer-endpoint
branch
from
6ce6c23 to
477db18
Compare
cprussin
force-pushed
the
cprussin/refactor-fortuna-explorer-endpoint
branch
from
477db18 to
94fd40b
Compare
This commit implements a few fixes for the fortuna explorer endpoint, and in doing so I refactored the query methods. We are now using a query builder approach which enables the filters, limits, offsets, etc to all work correctly regardless of the query supplied. This commit also implements a new query param to filter on state.
cprussin
force-pushed
the
cprussin/refactor-fortuna-explorer-endpoint
branch
from
94fd40b to
5df118e
Compare
cprussin
force-pushed
the
cprussin/refactor-fortuna-explorer-endpoint
branch
from
99a2d2f to
6e9c68a
Compare
| CREATE INDEX request__sequence__state__created_at ON request(sequence, state, created_at); | ||
| CREATE INDEX request__sequence__created_at ON request(sequence, created_at); | ||
| CREATE INDEX request__state__created_at ON request(state, created_at); | ||
| CREATE INDEX request__created_at ON request(created_at); |
There was a problem hiding this comment.
I got these indexes by using sqlite's cli expert. I did some testing to try to collapse the indices down to a smaller set, but it seems that reordering the columns does not result in indices that are properly reused where you might expect (i.e. I thought that if I swapped state and created_at in the index on line 11, that it would result in not needing the index on line 12, but testing this and checking query plan explain messages indicates that both are needed and reordering just fails to index properly the query that needs the first case).
| CREATE INDEX idx_request_sequence ON request (sequence); | ||
| CREATE INDEX idx_request_network_id_created_at ON request (network_id, created_at); | ||
| CREATE INDEX idx_request_created_at ON request (created_at); | ||
| CREATE INDEX idx_request_request_tx_hash ON request (request_tx_hash) WHERE request_tx_hash IS NOT NULL; |
There was a problem hiding this comment.
Are you sure this index was useless?
There was a problem hiding this comment.
What I did was to start by removing all indices and then work through every query combination with sqlite expert to generate the required list of indices to optimize all possible query combinations. I believe that it's not that these indices were useless, but more that the ones that are useful are superseded by more useful / efficient ones.
Summary
This commit implements a few fixes for the fortuna explorer endpoint, and in doing so I refactored the query methods. We are now using a query builder approach which enables the filters, limits, offsets, etc to all work correctly regardless of the query supplied.
This commit also implements a new query param to filter on state.
Rationale
The prior implementation of the history query methods did not properly support limit, offset, filters, etc on queries containing a search, which cause weird UX (I'd see results for a chain different than the one I selected, or searches based on sender would return too many results). Implementing this correctly led to a ton of duplication, so it seemed to make sense to move to a query builder pattern here.
How has this been tested?