Skip to content

feat: add batcher queue len and queue size in bytes metrics #1593

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.

Sign up for GitHub

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

Jump to bottom
Merged
JuArce merged 6 commits into from
Jan 8, 2025
Merged

feat: add batcher queue len and queue size in bytes metrics #1593

JuArce merged 6 commits into from
Jan 8, 2025

Conversation

@JulianVentura
Copy link
Contributor

@JulianVentura JulianVentura commented Dec 9, 2024

Add batcher queue len and queue size in bytes metrics

Description

This PR adds two new metrics for the batcher:

  • Queue length: The number of proofs waiting in the batcher's queue to be included in a batch
  • Queue size: The sum of the sizes of the verification data, measured in bytes, of the proofs waiting in the batcher's queue.
image

How to test

  1. Start all services locally, including the metrics
  2. Start sending proofs
  3. Go to the "System Data" dashboard and go to the bottom on the left size (the batcher side). You should see four new graphs showing the queue len and queue size.
  4. Stop sending proofs and the metrics should be reset back to zero.
  5. Send a burst of many proofs to see a spike in the graphs with make batcher_send_sp1_burst BURST_SIZE=23

Type of change

Please delete options that are not relevant.

  • New feature
  • Bug fix
  • Optimization
  • Refactor

Checklist

  • “Hotfix” to testnet, everything else to staging
  • Linked to Github Issue
  • This change depends on code or research by an external entity
    • Acknowledgements were updated to give credit
  • Unit tests added
  • This change requires new documentation.
    • Documentation has been added/updated.
  • This change is an Optimization
    • Benchmarks added/run
  • Has a known issue
  • If your PR changes the Operator compatibility (Ex: Upgrade prover versions)
    • This PR adds compatibility for operator for both versions and do not change batcher/docs/examples
    • This PR updates batcher and docs/examples to the newer version. This requires the operator are already updated to be compatible

MarcosNicolau
MarcosNicolau reviewed Dec 9, 2024
View reviewed changes
MarcosNicolau
MarcosNicolau reviewed Dec 9, 2024
View reviewed changes
Copy link
Member

@MarcosNicolau MarcosNicolau left a comment •
edited
Loading

Choose a reason for hiding this comment

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

You haven't updated the grafana json! Thou I added them myself and it does seem to be working fine.

JuArce
JuArce requested changes Dec 10, 2024
View reviewed changes
Copy link
Collaborator

@JuArce JuArce left a comment

Choose a reason for hiding this comment

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

There is missing to update the grafana dashbaord

uri-99
uri-99 suggested changes Dec 10, 2024
View reviewed changes
Copy link
Contributor

@uri-99 uri-99 left a comment

Choose a reason for hiding this comment

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

Lol yes missing graphana visualizations

@JulianVentura
Copy link
Contributor Author

JulianVentura commented Dec 10, 2024

Sorry! Grafana dashboard was just added @uri-99 @JuArce @MarcosNicolau

@JulianVentura JulianVentura self-assigned this Dec 10, 2024
@PatStiles
Copy link
Contributor

PatStiles commented Dec 10, 2024

Worked locally for me!

uri-99
uri-99 reviewed Dec 10, 2024
View reviewed changes
grafana/provisioning/dashboards/aligned/aggregator_batcher.json
Copy link
Contributor

@uri-99 uri-99 Dec 10, 2024

Choose a reason for hiding this comment

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

i think these 2 new boxes in grafana are quite informative, yet they ended up quite in the bottom of the dashboard. i would put them a bit higher

@JuArce JuArce added this pull request to the merge queue Jan 8, 2025
Merged via the queue into staging with commit 5d4c5e5 Jan 8, 2025
3 checks passed
@JuArce JuArce deleted the feat/batcher-queue-len-size-metric branch January 8, 2025 18:39
PatStiles pushed a commit that referenced this pull request Jan 9, 2025
@JulianVentura @PatStiles
feat: add batcher queue len and queue size in bytes metrics (#1593)
d6793c2 
Co-authored-by: Julian Ventura <[email protected]>
PatStiles pushed a commit that referenced this pull request Jan 9, 2025
@JulianVentura @PatStiles
feat: add batcher queue len and queue size in bytes metrics (#1593)
d3d8f87 
Co-authored-by: Julian Ventura <[email protected]>
PatStiles pushed a commit that referenced this pull request Jan 10, 2025
@JulianVentura @PatStiles
feat: add batcher queue len and queue size in bytes metrics (#1593)
0cdf2c6 
Co-authored-by: Julian Ventura <[email protected]>
PatStiles pushed a commit that referenced this pull request Jan 10, 2025
@JulianVentura @PatStiles
feat: add batcher queue len and queue size in bytes metrics (#1593)
42a1f9d 
Co-authored-by: Julian Ventura <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MarcosNicolau MarcosNicolau MarcosNicolau left review comments

@JuArce JuArce JuArce approved these changes

+2 more reviewers

@PatStiles PatStiles PatStiles approved these changes

@uri-99 uri-99 uri-99 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@JulianVentura JulianVentura

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@JulianVentura @PatStiles @uri-99 @JuArce @MarcosNicolau