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

REST RPC/Client Caching #4008

Closed
4 tasks
alexanderbez opened this issue Mar 29, 2019 · 8 comments
Closed
4 tasks

REST RPC/Client Caching #4008

alexanderbez opened this issue Mar 29, 2019 · 8 comments

Comments

@alexanderbez
Copy link
Contributor

alexanderbez commented Mar 29, 2019

Summary

As it stands, currently the REST client has no caching mechanisms or layers in place in either serving inbound requests or outbound RPC requests. Granted, the former can, and most often is, be done by a proxy service (e.g. nginx). However, the latter is not cached internally.

As the REST client evolves to become more complex in its data aggregation and servicing, I imagine it will need to start caching some of these outbound RPC requests. e.g. #4007

Note, I'm referring to caching RPC queries that query for content that is static (e.g blocks, txs, validator sets at a given height, etc...) and not Gaia querier requests.

Thoughts @jackzampolin @sabau @faboweb


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez alexanderbez changed the title REST Client Caching REST RPC/Client Caching Mar 29, 2019
@sabau
Copy link
Contributor

sabau commented Apr 2, 2019

If I understood the proposal is to build a caching system that seats between REST handlers and their interaction with the underlying RPC layer right?

So two queries should not produce two searches, but only the first one if (let's say) route is identical:
cliCtx.QueryWithData(route, nil)

If we keep it simple enough should be feasible to maintain, but we have to be careful, sometimes internal caching opens a pandora box.
But I would like to keep a flag to disable (or enable, depending on the default behaviour chosen) it.

@alexanderbez
Copy link
Contributor Author

alexanderbez commented Apr 2, 2019

If I understood the proposal is to build a caching system that seats between REST handlers and their interaction with the underlying RPC layer right?

Correct.

So two queries should not produce two searches, but only the first one if (let's say) route is identical:
cliCtx.QueryWithData(route, nil)

Not quite. Such a query can produce varying results depending on when they're executed (e.g. delegator rewards). As such, querier requests should not be cached -- only RPC calls (e.g. get tx, get block, get X).

If we keep it simple enough should be feasible to maintain, but we have to be careful, sometimes internal caching opens a pandora box.
But I would like to keep a flag to disable (or enable, depending on the default behaviour chosen) it.

Hmm, what kind of pandora's box will be opened? When would we want to disable caching?

@sabau
Copy link
Contributor

sabau commented Apr 2, 2019

Depending on which layer the cache is set it can eventually create some discussions:

  • how to handledisalignment of same primitive data cached on different places/routes at different times
  • invalidation rules: deciding when[/where[/cascading]] do replace it with fresher informations.
  • caching of simple data VS caching of already aggregated one?

Can I ask an example code-wise where the cache should reside?

Being able to disable it could be useful for development and testing where data is expected to change is very short time frames and we don't want to slow down the tests

@alexanderbez
Copy link
Contributor Author

alexanderbez commented Apr 2, 2019

There are no routes to cache really. We're only caching RPC calls from CLIContext.GetNode(). From what I can see, the calls we'd cache would be:

  • getBlock(height)
  • node.Validators(height)
  • node.Tx(hash, !cliCtx.TrustNode)
  • node.TxSearch(query, prove, page, limit)

So really only a few RPC calls would be cached. We'd have to handle verification as well.

@sabau
Copy link
Contributor

sabau commented Apr 5, 2019

I guess the first three will have no issues then.
The last one maybe need some extra care as two subsequent identical calls may differ if a tx happen in the meantime:

node.TxSearch(query, prove, page, limit)

@alessio
Copy link
Contributor

alessio commented Apr 15, 2019

What concerns me the most is cache invalidation for node.TxSearch during gaiacli execution as a web server.

@alexanderbez
Copy link
Contributor Author

This might be overkill for 2-3 RPC endpoints. Thinking we should close this for now.

@sabau
Copy link
Contributor

sabau commented Apr 23, 2019

If you agree we could address this, for now, by suggesting to people to use some proxy to cache their requests:

  • Publish on the docs some nginx configuration to suggest people to use (mine is a bit raw but easy to understand)
  • ask @greg-szabo and @mircea-c if they have some better tuned caching for the nodes

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

No branches or pull requests

3 participants