db: refactor race-build file cache reference tracking #4362
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions
file_cache.go line 113 at r1 (raw file):
nextRefID uint64 // openRefs maps reference IDs to the stack trace recorded at creation // time. It's used to track which call paths leaked open references to
[nit] double space
file_cache.go line 590 at r1 (raw file):
) (refID uint64, closeHook func(refID uint64)) { h.iterCount.Add(1) if invariants.RaceEnabled {
It would be a lot cleaner if we just generated a new closeHook here in race mode, so we don't have to plumb the refID around. I don't think the allocation is a problem in race mode, which is terribly slow anyway.
Previously the file cache used a pointer to the sstable iterator to maintain the stack traces of the origination of open references during race builds. This was a bit ambiguous. With the introduction of blob files, references will also be maintained by non-sstable iterators (eg, a cached blob file reader within an iterator's blob value fetcher). This commit refactors this reference tracking to allocate a new closure in race builds. The additional allocation is expected to be significant in (already slow) race builds.
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @RaduBerinde)
file_cache.go line 590 at r1 (raw file):
Previously, RaduBerinde wrote…
It would be a lot cleaner if we just generated a new
closeHookhere in race mode, so we don't have to plumb the refID around. I don't think the allocation is a problem in race mode, which is terribly slow anyway.
Genius
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2.
Reviewable status: all files reviewed (commit messages unreviewed), 2 unresolved discussions (waiting on @jbowens)
|
TFTR |
Previously the file cache used a pointer to the sstable iterator to maintain the stack traces of the origination of open references during race builds. This was a bit ambiguous. With the introduction of blob files, references will also be maintained by non-sstable iterators (eg, a cached blob file reader within an iterator's blob value fetcher).
This commit refactors this reference tracking to propagate a uint64 reference ID along with the closeHook, requiring the user to pass the reference ID back in their invocation of the closeHook. In production builds, this reference ID is always zero and is not stored. This hopefully will make usages unambiguous.