Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Marco Pracucci <marco@pracucci.com>
  • Loading branch information
pracucci committed Feb 1, 2021
1 parent fd639bb commit 4e71cae
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
16 changes: 8 additions & 8 deletions pkg/block/indexheader/lazy_binary_reader.go
Expand Up @@ -138,7 +138,7 @@ func (r *LazyBinaryReader) Close() error {
}

// Unload without checking if idle.
return r.unload(0)
return r.unloadIfIdleSince(0)
}

// IndexVersion implements Reader.
Expand Down Expand Up @@ -250,9 +250,9 @@ func (r *LazyBinaryReader) load() error {
return nil
}

// unload closes underlying BinaryReader if the reader is idle since idleSince time (as unix nano). If idleSince is 0,
// unloadIfIdleSince closes underlying BinaryReader if the reader is idle since given time (as unix nano). If idleSince is 0,
// the check on the last usage is skipped. Calling this function on a already unloaded reader is a no-op.
func (r *LazyBinaryReader) unload(idleSince int64) error {
func (r *LazyBinaryReader) unloadIfIdleSince(ts int64) error {
r.readerMx.Lock()
defer r.readerMx.Unlock()

Expand All @@ -261,8 +261,8 @@ func (r *LazyBinaryReader) unload(idleSince int64) error {
return nil
}

// Do not unload if not idle.
if idleSince > 0 && r.usedAt.Load() > idleSince {
// Do not unloadIfIdleSince if not idle.
if ts > 0 && r.usedAt.Load() > ts {
return errNotIdle
}

Expand All @@ -284,9 +284,9 @@ func (r *LazyBinaryReader) isLoaded() bool {
return r.reader != nil
}

// isIdle returns true if the reader is idle since idleSince time (as unix nano).
func (r *LazyBinaryReader) isIdle(idleSince int64) bool {
if r.usedAt.Load() > idleSince {
// isIdleSince returns true if the reader is idle since given time (as unix nano).
func (r *LazyBinaryReader) isIdleSince(ts int64) bool {
if r.usedAt.Load() > ts {
return false
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/block/indexheader/lazy_binary_reader_test.go
Expand Up @@ -204,14 +204,14 @@ func TestLazyBinaryReader_unload_ShouldReturnErrorIfNotIdle(t *testing.T) {
testutil.Equals(t, float64(0), promtestutil.ToFloat64(m.unloadFailedCount))

// Try to unload but not idle since enough time.
testutil.Equals(t, errNotIdle, r.unload(time.Now().Add(-time.Minute).UnixNano()))
testutil.Equals(t, errNotIdle, r.unloadIfIdleSince(time.Now().Add(-time.Minute).UnixNano()))
testutil.Equals(t, float64(1), promtestutil.ToFloat64(m.loadCount))
testutil.Equals(t, float64(0), promtestutil.ToFloat64(m.loadFailedCount))
testutil.Equals(t, float64(0), promtestutil.ToFloat64(m.unloadCount))
testutil.Equals(t, float64(0), promtestutil.ToFloat64(m.unloadFailedCount))

// Try to unload and idle since enough time.
testutil.Ok(t, r.unload(time.Now().UnixNano()))
testutil.Ok(t, r.unloadIfIdleSince(time.Now().UnixNano()))
testutil.Equals(t, float64(1), promtestutil.ToFloat64(m.loadCount))
testutil.Equals(t, float64(0), promtestutil.ToFloat64(m.loadFailedCount))
testutil.Equals(t, float64(1), promtestutil.ToFloat64(m.unloadCount))
Expand Down
8 changes: 4 additions & 4 deletions pkg/block/indexheader/reader_pool.go
Expand Up @@ -101,20 +101,20 @@ func (p *ReaderPool) Close() {
func (p *ReaderPool) closeIdleReaders() {
idleSince := time.Now().Add(-p.lazyReaderIdleTimeout).UnixNano()

for _, r := range p.getIdleReaders(idleSince) {
if err := r.unload(idleSince); err != nil && !errors.Is(err, errNotIdle) {
for _, r := range p.getIdleReadersSince(idleSince) {
if err := r.unloadIfIdleSince(idleSince); err != nil && !errors.Is(err, errNotIdle) {
level.Warn(p.logger).Log("msg", "failed to close idle index-header reader", "err", err)
}
}
}

func (p *ReaderPool) getIdleReaders(idleSince int64) []*LazyBinaryReader {
func (p *ReaderPool) getIdleReadersSince(ts int64) []*LazyBinaryReader {
p.lazyReadersMx.Lock()
defer p.lazyReadersMx.Unlock()

var idle []*LazyBinaryReader
for r := range p.lazyReaders {
if r.isIdle(idleSince) {
if r.isIdleSince(ts) {
idle = append(idle, r)
}
}
Expand Down

0 comments on commit 4e71cae

Please sign in to comment.