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

Create simple test for /stake/delegators/{delegatorAddr} #2307

Closed
4 tasks
NodeGuy opened this issue Sep 11, 2018 · 6 comments
Closed
4 tasks

Create simple test for /stake/delegators/{delegatorAddr} #2307

NodeGuy opened this issue Sep 11, 2018 · 6 comments
Assignees
Labels

Comments

@NodeGuy
Copy link
Contributor

NodeGuy commented Sep 11, 2018

Summary

Moved to the SDK repo from luniehq/lunie#1193.

Voyager has a critical performance bug—it takes 45 seconds to display information after a user click.

To fix the problem we need a test that we can use to measure the performance bottlenecks.

Problem Definition

The existing test that covers this endpoint, TestBonding, is complex (> 150 lines of code, creates non-deterministic state, exercises other REST calls, and tests 23 assertions), making it difficult to isolate the performance bottlenecks.

This is related to #2180 and #1553.

Proposal

Write new test function that:

  • is simple
  • creates its own node and Gaia Lite server
  • creates a state of one account delegating to another, nothing else
  • creates the state using the Go API, not REST calls
  • creates a deterministic state that's the same in every test, including the keys/addresses of both accounts

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

@NodeGuy sounds great! Few questions.

creates the state using the Go API, not REST calls

To do this, why do you then need to create a node and an LCD service? What benchmarks are you trying to capture? One thing you can immediately do to get a good starting sense, is to run a pprof. This will tell you where the biggest bottleneck is in the current test. We can go from there.

@NodeGuy
Copy link
Contributor Author

NodeGuy commented Sep 12, 2018

To do this, why do you then need to create a node and an LCD service?

We're consuming the endpoint so it should create an LCD service. I don't want extra REST calls executed to set up the state because then it's testing other endpoints inadvertently.

What benchmarks are you trying to capture?

@fedekunze can answer this question better than I.

@jackzampolin
Copy link
Member

Is there any chance @NodeGuy has some time to tackle this soon?? 🤞

@NodeGuy
Copy link
Contributor Author

NodeGuy commented Oct 15, 2018

I took a swing at this awhile ago with the help of @alessio and gave up. If I were to tackle it again I would take a different approach—moving the needle incrementally rather than doing The Right Thing. I don't think it's a priority for us any more but would defer to @faboweb here.

@faboweb
Copy link
Contributor

faboweb commented Oct 15, 2018

I think the endpoint is still slow, but I agree that this is not of priority right now. We can close the issue and reopen it, when we want to refactor the LCD tests.

@jackzampolin
Copy link
Member

@faboweb Like that approach. Going to go ahead and do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants