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

fix(cosmos-vstorage): split 'size_delta' counter into 'size_increase' and 'size_decrease' #11063

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

siarhei-agoric
Copy link
Contributor

@siarhei-agoric siarhei-agoric commented Feb 27, 2025

refs: #11062
refs: #10938

Description

Despite testing, telemetry.IncrCounterWithLabels() appears to only accept non-negative numbers. As a workaround, we will be reporting vstorage size increase and decrease as two separate counters.

Security Considerations

N/A

Scaling Considerations

One extra metric not an issue as long as number of keeper instances is low.

Documentation Considerations

New metrics:

# HELP store_size_increase store_size_increase
# TYPE store_size_increase counter
store_size_increase{storeKey="vstorage"} 42386

# HELP store_size_decrease store_size_decrease
# TYPE store_size_decrease counter
store_size_decrease{storeKey="vstorage"} 335

Testing Considerations

Manual Testing; Automated testing will be submitted as a separate PR.

First terminal:

cd agoric-sdk/packages/cosmic-swingset
make scenario2-setup scenario2-run-chain

Second terminal (after Block 17)

cd agoric-sdk/packages/cosmic-swingset
make fund-provision-pool
make provision-acct ACCT_ADDR=$(cat t1/8000/ag-cosmos-helper-address)

Third terminal (start before Block 17):

cd agoric-sdk/packages/cosmic-swingset
while true; do curl -s -G 'http://localhost:26660/metrics' | grep 'store_size_'; sleep 10; done
[...]
# HELP store_size_increase store_size_increase
# TYPE store_size_increase counter
store_size_increase{storeKey="vstorage"} 42386
# HELP store_size_decrease store_size_decrease
# TYPE store_size_decrease counter
store_size_decrease{storeKey="vstorage"} 335
# HELP store_size_increase store_size_increase
# TYPE store_size_increase counter
store_size_increase{storeKey="vstorage"} 284
# HELP store_size_decrease store_size_decrease
# TYPE store_size_decrease counter
store_size_decrease{storeKey="vstorage"} 335
# HELP store_size_increase store_size_increase
# TYPE store_size_increase counter
store_size_increase{storeKey="vstorage"} 284
# HELP store_size_decrease store_size_decrease
# TYPE store_size_decrease counter
store_size_decrease{storeKey="vstorage"} 788

Upgrade Considerations

N/A

Copy link
Member

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

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

The changes look good, but blocking until we have test coverage.

@siarhei-agoric siarhei-agoric changed the title fix(cosmos-vstorage): split 'size_delta' counter into 'allocated' and 'released' fix(cosmos-vstorage): split 'size_delta' counter into 'size_increase' and 'size_decrease' Feb 27, 2025
Copy link

cloudflare-workers-and-pages bot commented Feb 27, 2025

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 513ee68
Status: ✅  Deploy successful!
Preview URL: https://f715145d.agoric-sdk.pages.dev
Branch Preview URL: https://sliakh-11062-agd-panic-vstor.agoric-sdk.pages.dev

View logs

@siarhei-agoric siarhei-agoric marked this pull request as ready for review February 27, 2025 18:51
@siarhei-agoric siarhei-agoric requested a review from a team as a code owner February 27, 2025 18:51
Copy link
Member

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

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

Unblocking per the recorded manual testing and with the expectation of automated testing in a followup.

@siarhei-agoric
Copy link
Contributor Author

The changes look good, but blocking until we have test coverage.

I'd like to have this merged to get master fixed ASAP with just manual testing. A separate PR will be created for the automated testing.

@siarhei-agoric siarhei-agoric self-assigned this Feb 27, 2025
@siarhei-agoric siarhei-agoric added bug Something isn't working automerge:squash Automatically squash merge agd Agoric (Golang) Daemon labels Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agd Agoric (Golang) Daemon automerge:squash Automatically squash merge bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants