Skip to content

Conversation

@jan-auer
Copy link
Member

@jan-auer jan-auer commented Oct 17, 2018

Introduces a new model ProjectCfiCacheFile and generalizes cache generation over symcaches and CFI caches.

Based on #10052
Requires getsentry/symbolic#93

@jan-auer jan-auer changed the title Feat/cfi caches feat(native): Introduce CFI Caches Oct 17, 2018
@jan-auer jan-auer self-assigned this Oct 17, 2018
if e.errno != errno.ENOENT:
raise
symcache_file.cache_file.save_to(cachefile_path)
model.cache_file.save_to(cachefile_path)
Copy link
Member

Choose a reason for hiding this comment

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

noob question - How do local cache files get removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a periodically scheduled job clean_dsymcache that deletes files from the FS cache when they haven't been referenced for 1.5 days or longer. The actual "logic" is defined in this file at the very bottom: clear_old_entries.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you 🙇

@jan-auer
Copy link
Member Author

This is ready for review now. Since it's based off #10052, the first actual commit is 4076e2b. I'll rebase this as soon as we merge the other PR.

Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I have very little context on this section of the application.

def broken_make_symcache(self):
@classmethod
def broken_make_symcache(cls, obj):
raise SymbolicError('shit on fire')
Copy link
Member

Choose a reason for hiding this comment

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

👏

cficaches = ProjectDebugFile.difcache.get_cficaches(self.project, [debug_id])
assert debug_id in cficaches

def test_update_cficache(self):
Copy link
Member

Choose a reason for hiding this comment

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

Should there be a test for updating a cache file when the cache file has already been created by 'another' process?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, there should. I'm just not sure how to do this (as this can only happen for concurrent processes) and I don't want to mock the entire world. There will be a test for this soon, but for now please just believe me that this works :)

@jan-auer jan-auer merged commit 9a7d1c9 into master Oct 22, 2018
@jan-auer jan-auer deleted the feat/cfi-caches branch October 22, 2018 12:00
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants