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

fastsync: update the metrics during fast-sync #6590

Merged
merged 7 commits into from Jun 23, 2021

Conversation

JayT106
Copy link
Contributor

@JayT106 JayT106 commented Jun 16, 2021

Closes #3507

internal/consensus/reactor.go Outdated Show resolved Hide resolved
internal/consensus/reactor.go Outdated Show resolved Hide resolved
internal/consensus/state.go Outdated Show resolved Hide resolved
internal/consensus/reactor.go Outdated Show resolved Hide resolved
internal/consensus/state.go Show resolved Hide resolved
internal/blockchain/v0/reactor.go Outdated Show resolved Hide resolved
Copy link
Contributor

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

I think this is fine for now, Im not the biggest fan of passing the consensus state as we are doing.

This made me think that there may be some metrics that other reactors may want to touch, does it make sense to have node pkg metrics that various reactors can update depending on the various states of a node?

internal/blockchain/v0/reactor.go Outdated Show resolved Hide resolved
internal/consensus/state.go Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 17, 2021

Codecov Report

Merging #6590 (8ef165a) into master (6e238b5) will increase coverage by 0.02%.
The diff coverage is 37.50%.

❗ Current head 8ef165a differs from pull request most recent head 9517ea8. Consider uploading reports for the commit 9517ea8 to get more accurate results

@@            Coverage Diff             @@
##           master    #6590      +/-   ##
==========================================
+ Coverage   61.04%   61.07%   +0.02%     
==========================================
  Files         295      295              
  Lines       27968    27956      -12     
==========================================
  Hits        17074    17074              
+ Misses       9166     9157       -9     
+ Partials     1728     1725       -3     
Impacted Files Coverage Δ
internal/blockchain/v0/reactor.go 63.92% <0.00%> (-1.30%) ⬇️
internal/consensus/reactor.go 68.04% <0.00%> (+0.97%) ⬆️
internal/consensus/state.go 67.30% <60.00%> (-0.07%) ⬇️
internal/consensus/peer_state.go 78.94% <0.00%> (-1.76%) ⬇️
internal/p2p/transport_memory.go 84.21% <0.00%> (-1.32%) ⬇️
abci/client/socket_client.go 38.54% <0.00%> (-1.15%) ⬇️
internal/p2p/switch.go 60.18% <0.00%> (-0.47%) ⬇️
internal/p2p/router.go 81.58% <0.00%> (-0.24%) ⬇️
internal/statesync/reactor.go 58.71% <0.00%> (-0.16%) ⬇️
... and 12 more

@JayT106 JayT106 changed the title fastsync: update the metrics during fast-sync fastsync: update the metrics during fast-sync [DNM] Jun 17, 2021
@JayT106 JayT106 changed the title fastsync: update the metrics during fast-sync [DNM] fastsync: update the metrics during fast-sync Jun 22, 2021
@tac0turtle tac0turtle added C:consensus Component: Consensus C:sync Component: Fast Sync, State Sync T:observability Type: Observability T:ux Type: Issue or Pull Request related to developer experience labels Jun 23, 2021
@tac0turtle tac0turtle added the S:automerge Automatically merge PR when requirements pass label Jun 23, 2021
@mergify mergify bot merged commit 2b0a3c1 into tendermint:master Jun 23, 2021
@tac0turtle tac0turtle mentioned this pull request Jul 27, 2022
39 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:consensus Component: Consensus C:sync Component: Fast Sync, State Sync S:automerge Automatically merge PR when requirements pass T:observability Type: Observability T:ux Type: Issue or Pull Request related to developer experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sync: Prometheus metrics should update during fast_sync
4 participants