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

Converting lattice cache to use a durable key value store #513

Merged
merged 23 commits into from
Dec 14, 2022

Conversation

autodidaddict
Copy link
Member

@autodidaddict autodidaddict commented Dec 6, 2022

This adds the use of a durable key value store for metadata storage in place of the old LATTICECACHE_{prefix} stream.
This also deprecates the use of the linkdefs.put and linkdefs.del control interface operations, instead requiring clients to write directly to the key value bucket.

Upon startup, if a pre-existing (legacy) lattice cache is discovered, that will be read into memory, a deprecation warning will be emitted.

This version of the host does not write to the deprecated lattice cache, and all change discovery from the cache occurs through ephemeral consumers against the new WCMDCACHE_{prefix} key/value bucket.

Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
@ricochet
Copy link

ricochet commented Dec 7, 2022

How are consumers expected to migrate from legacy to WCMDCACHE (which my brain is reading as jibberish). What does WCMD standfor?

It seems possible to perform the migration in-place, i.e. instead of only reading into memory, call kv_put to the new bucket.

@autodidaddict
Copy link
Member Author

autodidaddict commented Dec 7, 2022

Wcmd is wasmCloud metadata. I may have gone too far in trying to ensure someone else won't already have a bucket with that name. I can simplify.

@autodidaddict
Copy link
Member Author

Thanks for pointing this out @ricochet , as written the code just caches in memory the old stream but never migrates to the new one.

@autodidaddict autodidaddict marked this pull request as draft December 7, 2022 12:48
@autodidaddict
Copy link
Member Author

Converting to a draft until I can make the appropriate changes

Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
@autodidaddict autodidaddict marked this pull request as ready for review December 7, 2022 15:56
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
@autodidaddict
Copy link
Member Author

New version of the code will, for each lattice prefix, locate a previously created latticecache_xxx stream, read its entire contents, put each element in the new KV bucket, and then delete the old latticecache_xxx stream.

Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
host_core/lib/host_core/claims/manager.ex Show resolved Hide resolved
host_core/lib/host_core/claims/manager.ex Show resolved Hide resolved
host_core/lib/host_core/jetstream/client.ex Outdated Show resolved Hide resolved
host_core/lib/host_core/jetstream/client.ex Show resolved Hide resolved
host_core/lib/host_core/jetstream/legacy_cache_loader.ex Outdated Show resolved Hide resolved
host_core/lib/host_core/jetstream/metadata_cache_loader.ex Outdated Show resolved Hide resolved
host_core/lib/host_core/jetstream/metadata_cache_loader.ex Outdated Show resolved Hide resolved
host_core/test/host_core/actors_test.exs Show resolved Hide resolved
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
connorsmith256
connorsmith256 previously approved these changes Dec 7, 2022
Copy link
Contributor

@connorsmith256 connorsmith256 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My questions are addressed, but someone with deeper knowledge of the current caching mechanisms should give a more thorough review

…wards compatible)

Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
def cache_claims(lattice_prefix, key, claims) do
:ets.insert(claims_table_atom(lattice_prefix), {key, claims})
end

def uncache_claims(lattice_prefix, key) do
:ets.delete(claims_table_atom(lattice_prefix), key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to consider a SafeEts module like we have SafeGnat request/pub functionality. I know that :ets operations will crash a genserver if the identifier doesn't refer to an existing table

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these tables are all guaranteed to be created before this stuff ever gets called. If the tables weren't created it means a mandatory supervisor never started, so we have bigger problems. however, I could add an implicit create in the call to something like claims_table_atom 🤔

host_core/lib/host_core/lattice/lattice_supervisor.ex Outdated Show resolved Hide resolved
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Signed-off-by: Kevin Hoffman <autodidaddict@users.noreply.github.com>
Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3/4 d'oh's later and it's great!

TL;DR @autodidaddict and I got on a call together and ran through some scenarios migrating from old (0.59.0 / pre-OTP refactor) scenarios and caught a few edge cases. Now the migration from older host configs and latticecache streams will be painless and older tools (like the older wash ctl clients) will work with linkdefs just fine.

@autodidaddict autodidaddict merged commit 65a4198 into main Dec 14, 2022
@autodidaddict autodidaddict deleted the feat/nats_metadata_cache branch December 14, 2022 19:17
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

Successfully merging this pull request may close these issues.

None yet

4 participants