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

Completions caching #6

Open
mriehl opened this issue Sep 2, 2020 · 7 comments
Open

Completions caching #6

mriehl opened this issue Sep 2, 2020 · 7 comments

Comments

@mriehl
Copy link

mriehl commented Sep 2, 2020

The oracle databases I currently work with unfortunately have a ton of namespaces and tables.
This results in "Refreshing completions" for around a minute every time I start okcli.

I've talked a bit with @icepuma about this issue and we had the idea to add optional completion caching to okcli.
It could work like this:

  • entirely opt-in and set by command line flag (eG --enable-completions-cache or similar)
  • if enabled, the completions are serialized into the XDG_CACHE_DIR
  • if enabled and there is a completions dump at start, deserialize from file instead of asking the DB
  • if disabled, nothing changes

The cache would be per-connection, for example after hashing the connection string.
I don't have any particular preference regarding cache invalidation, maybe simply removing the file manually would be good enough for a first start, or a --force-invalidate-cache option (slightly better especially if the file name is a hash).

A slightly easier variation to avoid dealing with XDG would be to simply support --completion-cache <path> so the user can decide if and when to delete the file. But it also introduces the potential mistake of using a completion cache for a different connection which would be confusing.

What are your thoughts on this and would you accept a pull request implementing this?

@cboddy
Copy link
Collaborator

cboddy commented Sep 2, 2020

Thanks for raising this.

A completion cache would be a useful addition for your environment and I'm definitely open to reviewing a PR for it.


    entirely opt-in and set by command line flag (eG --enable-completions-cache or similar)
    if enabled, the completions are serialized into the XDG_CACHE_DIR
    if enabled and there is a completions dump at start, deserialize from file instead of asking the DB
    if disabled, nothing changes

The cache would be per-connection, for example after hashing the connection string.
I don't have any particular preference regarding cache invalidation, maybe simply removing the file manually would be good enough for a first start, or a --force-invalidate-cache option (slightly better especially if the file name is a hash).

This sounds sensible to me, here are a few thoughts:

  • This should also be a flag that can be set in the config file.
  • For invalidating the cache: since the completions-loading is async we can simply reload them from the db-host on each new connection and update the cache (ie. use the cached-schema until the the schemas are loaded and then swap them). This is possibly more tailored to larger orgs/teams where DDL changes can be quite common but I think that's probably the average for Oracle DB.
  • Using XDG_CACHE_DIR is OK but in principle we'd like to be compatible with Windows (although AFAIK no one is using okcli on that platform yet) so how about if it falls back to serialising them to the directory where it has loaded the okcli config from (unless explicitly set as a parameter) if XDF_CACHE_DIR is undefined?
  • Just to confirm: when you say per-connection caching, do you mean each db-connection string? It should also work with tnsnames.

Happy to discuss further, otherwise look forward to a PR!

@icepuma
Copy link

icepuma commented Sep 2, 2020

Caching per tnsname would be the thing I guess e.g. cache key is <XXX> in <user>/<pass>@<XXX>.

@cboddy
Copy link
Collaborator

cboddy commented Sep 3, 2020

Caching per tnsname would be the thing I guess e.g. cache key is in /@.

Good idea, it should also include the user since they may have different read permissions - so something like hash(user || tnsname).

@mriehl
Copy link
Author

mriehl commented Sep 3, 2020

Sounds good, I'll start working on this ASAP.

@mriehl
Copy link
Author

mriehl commented Sep 21, 2020

@cboddy if you get a chance can you take a look at #7?

@cboddy
Copy link
Collaborator

cboddy commented Sep 22, 2020

@mriehl will do tomorrow, thanks.

@ASC8384
Copy link

ASC8384 commented Mar 26, 2021

Today is 2021, has it come true?
Thanks for your work again.

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

No branches or pull requests

4 participants