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

Implement transaction metrics query in new Rust backend #274

Conversation

Victor-N-Suadicani
Copy link
Contributor

@Victor-N-Suadicani Victor-N-Suadicani commented Oct 21, 2024

Purpose

The current transaction metrics query is broken. It times out when choosing the yearly period. We should fix this with the new backend.

Changes

  • Creates a new transactions_metrics module which will contain the logic for this query. This also serves as an example of how we can split up the logic in different modules more generally. Draft for now to show this.
  • Adds an index column to the transactions database table (renaming the previous one to block_index) which will be a globally unique index and serves as the new primary key.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

Base automatically changed from rust-backend to main October 24, 2024 09:51
@Victor-N-Suadicani Victor-N-Suadicani marked this pull request as ready for review October 28, 2024 12:19
@Victor-N-Suadicani
Copy link
Contributor Author

I implemented the transaction metrics query now. It should work and should be efficient, though I haven't tested with data in the database. EXPLAIN ANALYZE suggests that it is fast with the added slot_time index though.

The behavior is not exactly the same either. The old behavior put the buckets at certain times, which lead to the last bucket covering into the future, which is strange and means that the last bucket is actually almost always smaller than the rest. The new behavior has the last bucket covering just up until the current moment. This makes the buckets even.

@limemloh
Copy link
Contributor

The behavior is not exactly the same either. The old behavior put the buckets at certain times, which lead to the last bucket covering into the future, which is strange and means that the last bucket is actually almost always smaller than the rest. The new behavior has the last bucket covering just up until the current moment. This makes the buckets even.

This would mean that it behaves differently from the other metrics we have, and we would need to change those as well.
Not sure if this is better though, since the buckets shift in time, the graphs drawn from these metrics will probably start looking like waves as the count in each block shifts from one bucket to the next.

@Victor-N-Suadicani
Copy link
Contributor Author

Victor-N-Suadicani commented Oct 28, 2024

It's a fair point but I personally feel that's okay. Yes, every bucket would change depending on when you look, but at least the last bucket doesn't look weird. The current solution makes it seem like the last bucket almost always has less activity, which is inaccurate. I feel this is the most accurate way to portray the data, at least if we are talking about a bar chart with buckets. A finer-resolution line chart would not have the same problem to the same extent I think?

One way to fix it would be to fix the buckets in a similar manner as before, but only consider bucket that do not overlap with the future. But then you don't see the most up-to-date data. I feel the solution proposed here is better.

Side note, I ran the indexer for stagenet for a while and tested the query on the resulting data and it seems to function as intended.

backend-rust/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@limemloh limemloh left a comment

Choose a reason for hiding this comment

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

Let's stick to this for now then, we can then evaluate the end result when we have more of the backend ready and start connecting the frontend.

@Victor-N-Suadicani Victor-N-Suadicani merged commit f80c5e3 into main Oct 29, 2024
4 checks passed
@Victor-N-Suadicani Victor-N-Suadicani deleted the vs/ccd-106-ccdscan-fix-timeout-transactions-page-when-a-year-queried branch October 29, 2024 08:24
@DOBEN
Copy link
Member

DOBEN commented Oct 29, 2024

Approved from my side :)

@Victor-N-Suadicani Victor-N-Suadicani mentioned this pull request Oct 29, 2024
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.

3 participants