From e049f8edb4d1f54b39bf582630323577241c11e0 Mon Sep 17 00:00:00 2001 From: Kern Walster Date: Thu, 24 Apr 2025 23:19:58 +0000 Subject: [PATCH] Pass cache options to spancache.Get Before this change, the span manager did not pass cache opts to the span cache when getting keys, but it did pass options when putting keys. This change makes them consistent. This is problematic with the Direct option because writes are directly to files, but reads use a file descriptor cache. This leads to the following issue: 1. The background fetcher fetches a span, but does not decompress it 2. The background fetcher writes the compressed span to the span cache 3. A later read loads the compressed data from the span cache. This caches the file descriptor 4. The span is decompressed 5. The span is rewritten to the same cache key by opening a temp file, writing the decompressed span data, then renaming it to the original name. NOTE: This is a different file descriptor from step 3! 6. The span is marked decompressed 7. Another read hits in the cache and get's back the fd to the (now deleted) compressed file from step 3. 8. SOCI returns compressed data or get's an unxpected EOF because there isn't as much compressed data as uncompressed data. The solution here is to pass cache options. Each cache Get opens a new file descriptor, so the caching issue is bypassed. Signed-off-by: Kern Walster --- fs/span-manager/span_manager.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/span-manager/span_manager.go b/fs/span-manager/span_manager.go index 3b82a524a..9c3ab86c6 100644 --- a/fs/span-manager/span_manager.go +++ b/fs/span-manager/span_manager.go @@ -270,7 +270,7 @@ func (m *SpanManager) getSpanContent(spanID compression.SpanID, offsetStart, off } // cache uncompressed span - if err := m.addSpanToCache(s.id, uncompSpanBuf, m.cacheOpt...); err != nil { + if err := m.addSpanToCache(s.id, uncompSpanBuf); err != nil { return nil, err } if err := s.setState(uncompressed); err != nil { @@ -328,7 +328,7 @@ func (m *SpanManager) fetchAndCacheSpan(spanID compression.SpanID, uncompress bo } // cache span data - if err := m.addSpanToCache(spanID, buf, m.cacheOpt...); err != nil { + if err := m.addSpanToCache(spanID, buf); err != nil { return nil, err } if err := s.setState(state); err != nil { @@ -389,8 +389,8 @@ func (m *SpanManager) uncompressSpan(s *span, compressedBuf []byte) ([]byte, err // addSpanToCache adds contents of the span to the cache. // A non-nil error is returned if the data is not written to the cache. -func (m *SpanManager) addSpanToCache(spanID compression.SpanID, contents []byte, opts ...cache.Option) error { - w, err := m.cache.Add(fmt.Sprintf("%d", spanID), opts...) +func (m *SpanManager) addSpanToCache(spanID compression.SpanID, contents []byte) error { + w, err := m.cache.Add(fmt.Sprintf("%d", spanID), m.cacheOpt...) if err != nil { return err } @@ -410,7 +410,7 @@ func (m *SpanManager) addSpanToCache(spanID compression.SpanID, contents []byte, // `offset` is the offset of the requested contents within the span. // `size` is the size of the requested contents. func (m *SpanManager) getSpanFromCache(spanID compression.SpanID, offset, size compression.Offset) (io.ReadCloser, error) { - rc, err := m.cache.Get(fmt.Sprintf("%d", spanID)) + rc, err := m.cache.Get(fmt.Sprintf("%d", spanID), m.cacheOpt...) if err != nil { return nil, fmt.Errorf("%w: %w", ErrSpanNotAvailable, err) }