-
Notifications
You must be signed in to change notification settings - Fork 885
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
Switch OSD Optimizer to rely on file hashes instead of mtime
s
#8472
Switch OSD Optimizer to rely on file hashes instead of mtime
s
#8472
Conversation
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
readFile$(path).pipe( | ||
map( | ||
(buffer) => | ||
[path, Crypto.createHash('sha1').update(buffer).digest('base64')] as const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Core change is this, the rest is plumbing -- most "get the file modification ID" checks trace back to here (except one occurrence in run_compiler.ts
which also was updated).
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8472 +/- ##
==========================================
- Coverage 60.94% 60.93% -0.01%
==========================================
Files 3759 3759
Lines 89329 89330 +1
Branches 13973 13973
==========================================
- Hits 54438 54437 -1
- Misses 31494 31495 +1
- Partials 3397 3398 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov: this code is hit in every functional test workflow multiple times |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
* Hotswap getMtimes to compute sha1 hashes Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * Rename all the things Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * Changeset file for PR #8472 created/updated --------- Signed-off-by: Simeon Widdis <sawiddis@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 0609ecf) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
… (#8532) * Hotswap getMtimes to compute sha1 hashes * Rename all the things * Changeset file for PR #8472 created/updated --------- (cherry picked from commit 0609ecf) Signed-off-by: Simeon Widdis <sawiddis@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
…search-project#8472) * Hotswap getMtimes to compute sha1 hashes Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * Rename all the things Signed-off-by: Simeon Widdis <sawiddis@amazon.com> * Changeset file for PR opensearch-project#8472 created/updated --------- Signed-off-by: Simeon Widdis <sawiddis@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
The current build cache in OSD optimizer depends on tracking the modification times of previous files. This means it never recognizes cached build artifacts in CI, because the artifacts get "updated" every time code is pulled. Refactoring the entire cache system to use file content hashes instead of modification times results in a minor speedup in build times, in cases where the build artifacts are cached. See #8428 for more information.
As followup, I have some discussion in opensearch-project/dashboards-observability#2188 regarding how best to actually use the cached artifacts.
Issues Resolved
Resolves #8428
Unblocks opensearch-project/dashboards-observability#2188
Screenshot
Testing the changes
Described in #8428: I set up build runs with verbose compilation enabled and tracked the built packages over time in the case of no cache, an old cache hit, and a new cache hit. Here's the results:
I also checked what breaks when pulling this on top of the old build system. Aside from a full recompilation (every file is now invalid), it doesn't seem to have issues when migrating from the old version -- if there's issues then ye olde
yarn osd clean
should do the trick.Changelog
mtime
sCheck List
yarn test:jest
yarn test:jest_integration