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.

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

[CLI][Localnet][Bug] - [localnet_client_debug] Error 137 #607

Closed
9 tasks
deblasis opened this issue Mar 23, 2023 · 0 comments · Fixed by #608
Closed
9 tasks

[CLI][Localnet][Bug] - [localnet_client_debug] Error 137 #607

deblasis opened this issue Mar 23, 2023 · 0 comments · Fixed by #608
Assignees
@deblasis
Labels
bug Something isn't working - expected behaviour is incorrect client work needed to interface with the node (rpc, cli, etc..) infra Core infrastructure - not protocol related
Milestone
M1: Pocket PoS (P...

Comments

@deblasis
Copy link
Contributor

Objective

Currently, the debug keybase is rehydrated from the private_keys.yaml that is embedded in the binary if it's built with the "debug" build tag.

It works but it means that the rehydration process has to be performed every time the local keybase is emptied and that can happen a lot especially when developing with quick feedback loops in localnet with lots of make localnet_down followed by localnet_up.

We should improve this because it causes frustrations and slows down development.

Origin Document

Multiple conversations and first-hand experience. Just a fresh example:

image

I am also still experiencing random:

{"level":"debug","time":"2023-03-17T18:51:00Z","message":"Debug keybase initializing... Adding 999 validator keys to the keybase"}
command terminated with exit code 137
make: *** [localnet_client_debug] Error 137

Goals

  • Optimize the process

Deliverable

  • Updated codebase

Non-goals / Non-deliverables

  • Logic updates

General issue deliverables

  • Update the appropriate CHANGELOG(s)
  • Update any relevant local/global README(s)
  • Update relevant source code tree explanations
  • Add or update any relevant or supporting mermaid diagrams

Testing Methodology

  • Videos showing the improvement
  • New tests or benchmarks: make ...
  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md

Creator: @deblasis
Co-Owners: @gokutheengineer, @okdas
@deblasis deblasis added bug Something isn't working - expected behaviour is incorrect infra Core infrastructure - not protocol related client work needed to interface with the node (rpc, cli, etc..) labels Mar 23, 2023
@deblasis deblasis self-assigned this Mar 23, 2023
@deblasis deblasis moved this to In Progress in V1 Dashboard Mar 23, 2023
@deblasis deblasis mentioned this issue Mar 23, 2023
Merged
17 tasks
deblasis added a commit that referenced this issue Mar 24, 2023
@deblasis @h5law
[CLI][Localnet][Bug] - [localnet_client_debug] Error 137 - Issue #607 (…
b12811d 
…#608)

## Description

This PR improves the keybase initialization by:
- doing the work only if the `private_keys.yaml` has been changed 
- creating a backup of the keybase
- loading it (which is way cheaper than parsing the yaml and rehydrating
the keybase from scratch)


### Before

At 1:46 I run `make localnet_client_debug` and you can see it takes a
long time plus it's a very expensive operation


https://user-images.githubusercontent.com/29378614/227202479-1320fdf4-f851-4a58-884e-a36b3c385b84.mp4



### After

At 1:12 I run `make localnet_client_debug` and it's basically
instantaneous with no further spikes in CPU/RAM usage


https://user-images.githubusercontent.com/29378614/227202494-a1eff819-e2d9-4881-a2e5-4b28448d574c.mp4

Where is the magic? 
We do the expensive operation only when needed and that means when the
`private_keys.yaml` has changed.

This is handled by a small binary that calculates the hash of the yaml
file, compares it with the one we have already (if any) and if needed,
it recreates a backup of the keybase that can be rehydrated from the
debug client.

Sloppy recording but it shows the whole process:


https://user-images.githubusercontent.com/29378614/227204654-7b7f3ea8-6db5-473c-ac6c-75be13f60553.mp4

## Issue

Fixes #607 

## Type of change

Please mark the relevant option(s):

- [x] New feature, functionality or library
- [x] Bug fix
- [ ] Code health or cleanup
- [ ] Major breaking change
- [ ] Documentation
- [ ] Other <!-- add details here if it a different type of change -->

## List of changes

- Updated debug keybase initialization
- Created small binary to help with the integrity check

## Testing

- [x] `make develop_test`
- [x]
[LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md)
w/ all of the steps outlined in the `README`

<!-- REMOVE this comment block after following the instructions
 If you added additional tests or infrastructure, describe it here.
 Bonus points for images and videos or gifs.
-->

## Required Checklist

- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added, or updated, [`godoc` format
comments](https://go.dev/blog/godoc) on touched members (see:
[tip.golang.org/doc/comment](https://tip.golang.org/doc/comment))
- [x] I have tested my changes using the available tooling
- [x] I have updated the corresponding CHANGELOG

### If Applicable Checklist

- [ ] I have updated the corresponding README(s); local and/or global
- [x] I have added tests that prove my fix is effective or that my
feature works
- [ ] I have added, or updated,
[mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding
README(s)
- [ ] I have added, or updated, documentation and
[mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*`
if I updated `shared/*`README(s)

---------

Co-authored-by: harry <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in V1 Dashboard Mar 24, 2023
bryanchriswhite added a commit that referenced this issue Mar 28, 2023
@bryanchriswhite
Merge remote-tracking branch 'pokt/main' into refactor/consolidate-p2…
2770f41 
…p-modules

* pokt/main:
  [Utility][Persistence][Savepoints/Rollbacks] Refactor UtilityContext into UtilityUnitOfWork (Issue #563) (#577)
  [Consensus] Add Quorum Certificate to the block (#593)
  docs(wiki): ValueError: Duplicate wiki path: devlog/template (#620)
  Remove unused import
  [Infra][P2P] Bug: "address not found in peerstore" error in localnet_debug_client when scaling up Localnet - Issue #604 (#611)
  [CLI][Localnet][Bug] - [localnet_client_debug] Error 137 - Issue #607 (#608)
  [Documentation] Fix links to RPC module docs in README (#603)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@deblasis deblasis

Labels
bug Something isn't working - expected behaviour is incorrect client work needed to interface with the node (rpc, cli, etc..) infra Core infrastructure - not protocol related
Projects
V1 Dashboard
Status: Done
Milestone
M1: Pocket PoS (Proof of Stake)
Development

Successfully merging a pull request may close this issue.

[CLI][Localnet][Bug] - [localnet_client_debug] Error 137 - Issue #607 pokt-network/pocket
1 participant
@deblasis