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

[Dashboard] http route handler cache #10921

Merged
merged 6 commits into from
Oct 10, 2020

Conversation

fyrestone
Copy link
Contributor

Why are these changes needed?

  • Problem
    • The new dashboard needs a cache to avoid unnecessary calculation.
  • Solution
    • An aiohttp_cache decorator for aiohttp route handler.
      • If the same request arrives in TTL, just return the cached data.
      • If a new request arrives or the TTL is expired, schedule the http route handler to update cache, and return the cached data.
      • Fixed size with LRU replacement policy, default is 128 (same as functools.lru_cache 's default maxsize).
      • An environment var RAY_DASHBOARD_NO_CACHE(set before the new dashboard starts) to disable cache.
  • Other
    • Make the rest_response a normal function (same as aiohttp.web.json_response) instead of an async function.

Related issue number

Closes #10771

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@fyrestone fyrestone added the dashboard Issues specific to the Ray Dashboard label Sep 21, 2020
@fyrestone fyrestone modified the milestone: New Dashboard Sep 21, 2020
@rkooo567
Copy link
Contributor

rkooo567 commented Sep 23, 2020

@fyrestone Btw, do you mind writing simple documentation about our dashboard framework (like basic how this works and how to use it)? It'll be helpful for future developers who will work on the dashboard. FUN FACT: I also am not familiar how the whole thing works...

@fyrestone
Copy link
Contributor Author

@fyrestone Btw, do you mind writing simple documentation about our dashboard framework (like basic how this works and how to use it)? It'll be helpful for future developers who will work on the dashboard. FUN FACT: I also am not familiar how the whole thing works...

OK. I will write a document about the new dashboard framework.

@rkooo567
Copy link
Contributor

cc @mfitton Please review the PR!

@rkooo567
Copy link
Contributor

rkooo567 commented Oct 2, 2020

@fyrestone Please resolve the merge conflict!

@rkooo567 rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Oct 2, 2020
@fyrestone fyrestone added tests-ok The tagger certifies test failures are unrelated and assumes personal liability. and removed @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. labels Oct 9, 2020
@fyrestone
Copy link
Contributor Author

@fyrestone Please resolve the merge conflict!

Thanks. The conflict has been resolved.

@rkooo567 rkooo567 merged commit defd41a into ray-project:master Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dashboard Issues specific to the Ray Dashboard tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard][Perf] http route handler cache
3 participants