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

[inferno-vc] Made fetchObjectClosureHashes return the root scriptId and added logging to cached client and server #137

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

albertov
Copy link
Contributor

Made fetchObjectClosureHashes return the root scriptId because it belongs to the closure and so it is coherent with fetchObjectClosure. This makes the cached client take a lock on the requested scriptHash too when fetching the object which it wasn't doing before.

Also added logging to cached client to see hits/missed and to server to see which scriptIds are being used to call fetchObjects and fetchObjectClosureHashes

Copy link
Contributor

@mirko-plowtech mirko-plowtech left a comment

Choose a reason for hiding this comment

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

LGTM!

@mirko-plowtech mirko-plowtech merged commit 8cc5e91 into main Oct 11, 2024
1 check passed
@mirko-plowtech mirko-plowtech deleted the albertov_fix_cached_client branch October 11, 2024 16:16
@brent80 brent80 restored the albertov_fix_cached_client branch October 11, 2024 21:53
brent80 added a commit that referenced this pull request Oct 11, 2024
…riptId and added logging to cached client and server" (#138)

Reverts #137
brent80 pushed a commit that referenced this pull request Oct 12, 2024
…nd added logging to cached client and server (take 2) (#139)

Second take of #137. This version:

- removes emojis from log messages so `LANG` env var doesn't need to be
set to a UTF8 compatible one
- Changes the directory where closure hashes are cached to `deps-v2` so
we don't read previously cached deps without the root scriptId
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.

2 participants