Repository: influxdata/influxdb
File: tsdb/engine/tsm1/file_store.go (Go)

Code before:
				logger.Warn("failed to remove", zap.Int("files", failCount))
			}
			logEndOp()
		}()

		// hasFiles() acquires the lock to check files.Len() and, if empty,
		// sets running = false before returning. This ensures no race between
		// add() checking running and the goroutine exiting.
		for p.hasFiles() {
			p.files.Range(func(k string, v TSMFile) bool {
				// In order to ensure that there are no races with this (file held externally calls Ref
				// after we check InUse), we need to maintain the invariant that every handle to a file
				// is handed out in use (Ref'd), and handlers only ever relinquish the file once (call Unref
				// exactly once, and never use it again).
				if !v.InUse() {
					if err := v.Close(); err != nil {
						logger.Error("close file failed", zap.String("file", k), zap.Error(err))
						failCount++
					} else if err := v.Remove(); err != nil {
						logger.Error("remove file failed", zap.String("file", k), zap.Error(err))
						failCount++
					} else {
						logger.Debug("successfully removed", zap.String("file", k))
						purgeCount++
					}
					// Remove the file regardless of success or failure.
					// Do not retry files which could not be closed or removed.
					p.files.Delete(k)
				}
				// InUse files are left to be tried later.
				return true
			})

			time.Sleep(time.Second)
		}
	}()
}

func (p *purger) hasFiles() bool {
	p.mu.Lock()
	defer p.mu.Unlock()

	if p.files.Len() == 0 {
		p.running = false
		return false
	}
	return true
}

type tsmReaders []TSMFile


Review comment (bug):
Are you thinking we shouldn't retry because we're just likely to fail again (and therefore forever)? The purger is already the second attempt to remove a file (if i recall correctly) so not retrying and retrying makes sense to me. 

I'm curious the why behind the change: why don't you want to retry anymore? The comment says what it does but not why.

Code after:
			}
			logEndOp()
		}()

		// hasFiles() acquires the lock to check files.Len() and, if empty,
		// sets running = false before returning. This ensures no race between
		// add() checking running and the goroutine exiting.
		for p.hasFiles() {
			p.files.Range(func(k string, v TSMFile) bool {
				// In order to ensure that there are no races with this (file held externally calls Ref
				// after we check InUse), we need to maintain the invariant that every handle to a file
				// is handed out in use (Ref'd), and handlers only ever relinquish the file once (call Unref
				// exactly once, and never use it again).
				if !v.InUse() {
					if err := v.Close(); err != nil {
						logger.Error("close file failed", zap.String("file", k), zap.Error(err))
						failCount++
					} else if err := v.Remove(); err != nil {
						logger.Error("remove file failed", zap.String("file", k), zap.Error(err))
						failCount++
					} else {
						logger.Debug("successfully removed", zap.String("file", k))
						purgeCount++
					}
					// Remove the file regardless of success or failure.
					// Do not retry files which could not be closed or removed.
					// This is because they have already been closed, or there
					// is an operating system problem that is unlikely to
					// resolve by itself.
					p.files.Delete(k)
				}
				// InUse files are left to be tried later.
				return true
			})

			time.Sleep(time.Second)
		}
	}()
}

func (p *purger) hasFiles() bool {
	p.mu.Lock()
	defer p.mu.Unlock()

	has := false

	// Avoid calling Len() which iterates over the whole map.
	p.files.Range(func(k string, v TSMFile) bool {
		has = true
		return false // stop iteration after finding the first file
	})