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

Transient accounts hash cache dir is unnecessary #33181

Merged

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Sep 7, 2023

Problem

We create a subdirectory within the accounts hash cache path to hold transient files created while performing the accounts hash calculation. These files are temporary and unnamed, and are not visible by a user looking at the filesystem, so a persistent user-visible directory is not necessary.

Summary of Changes

Remove the persistent transient accounts hash cache dir; instead create a temp dir.

@brooksprumo brooksprumo self-assigned this Sep 7, 2023
@brooksprumo brooksprumo force-pushed the accounts-hash-cache/transient branch from 501abb4 to 1246025 Compare September 7, 2023 16:33
@brooksprumo brooksprumo added work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Sep 7, 2023
@codecov
Copy link

codecov bot commented Sep 7, 2023

Codecov Report

Merging #33181 (51bdb36) into master (a145ade) will decrease coverage by 0.2%.
Report is 11 commits behind head on master.
The diff coverage is 90.3%.

@@            Coverage Diff            @@
##           master   #33181     +/-   ##
=========================================
- Coverage    82.1%    81.9%   -0.2%     
=========================================
  Files         786      789      +3     
  Lines      211606   213444   +1838     
=========================================
+ Hits       173812   174962   +1150     
- Misses      37794    38482    +688     

@brooksprumo brooksprumo force-pushed the accounts-hash-cache/transient branch from 1246025 to 51bdb36 Compare September 11, 2023 19:58
@brooksprumo brooksprumo added CI Pull Request is ready to enter CI and removed work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Sep 11, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Sep 11, 2023
@brooksprumo brooksprumo marked this pull request as ready for review September 11, 2023 20:29
@brooksprumo brooksprumo added the v1.16 PRs that should be backported to v1.16 label Sep 11, 2023
Copy link
Contributor

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm

@brooksprumo brooksprumo merged commit c61ee20 into solana-labs:master Sep 12, 2023
@brooksprumo brooksprumo deleted the accounts-hash-cache/transient branch September 12, 2023 15:23
mergify bot pushed a commit that referenced this pull request Sep 12, 2023
(cherry picked from commit c61ee20)

# Conflicts:
#	runtime/src/accounts_db.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.16 PRs that should be backported to v1.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants