Skip to content

Commit 04faaea

Browse files
committed
Resolve Mergify backport conflicts on 8.19
The automated cherry-pick of #51863 left conflict markers in fswatch.go and fswatch_test.go and pulled in main-only APIs (harvester progress metrics, the GetFiles options/metrics signature, completedFingerprints) that do not exist on 8.19. Reapply only the two allocation optimizations relevant to this branch, adapted to its filestream APIs: - fileWatcher.watch resolves the file identity (srcID) lazily via an ensureSrcID closure; the closed-harvester reconciliation is split into hasClosedHarvesters/reconcileClosedHarvester so an idle scan never resolves an identity. - fileScanner.GetFiles pre-sizes its per-scan maps from the previous scan's unique-file count via a new lastCount field. Adapt the new BenchmarkWatchIdle to the branch's watch/newFileWatcher signatures.
1 parent 28c90e8 commit 04faaea

2 files changed

Lines changed: 61 additions & 222 deletions

File tree

filebeat/input/filestream/fswatch.go

Lines changed: 22 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -164,19 +164,7 @@ func (w *fileWatcher) processNotification(evt loginp.HarvesterStatus) {
164164
func (w *fileWatcher) watch(ctx unison.Canceler) {
165165
w.log.Debug("Start next scan")
166166

167-
<<<<<<< HEAD
168167
paths := w.scanner.GetFiles()
169-
=======
170-
// file identity is updated in GetFiles
171-
now := time.Now()
172-
scanOpts := loginp.FileScanOptions{
173-
CurrentTime: now,
174-
IgnoreOlder: ignoreOlder,
175-
IgnoreInactiveSince: ignoreInactiveSince,
176-
}
177-
paths, scanMetrics := w.scanner.GetFiles(scanOpts)
178-
metrics.UpdateFileScanMetrics(scanMetrics)
179-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
180168

181169
// for debugging purposes
182170
writtenCount := 0
@@ -199,8 +187,9 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
199187
continue
200188
}
201189

202-
// srcID is the file identity (harvester ID/registry key), resolved lazily via ensureSrcID:
203-
// an unchanged, untracked file (gzip, empty, ignore_older) never needs one, saving allocs.
190+
// srcID is the file identity (harvester ID/registry key), resolved lazily via
191+
// ensureSrcID: an unchanged file that produces no event never needs one, saving a
192+
// per-file identity allocation on every idle scan.
204193
var srcID string
205194
ensureSrcID := func() string {
206195
if srcID == "" { // getFileIdentity never returns ""
@@ -209,7 +198,8 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
209198
return srcID
210199
}
211200

212-
// closedHarvesters is empty in the steady state; this reconciliation is usually skipped.
201+
// closedHarvesters is empty in the steady state, so this reconciliation is usually
202+
// skipped and the file identity is never resolved.
213203
if w.hasClosedHarvesters() {
214204
w.reconcileClosedHarvester(&prevDesc, ensureSrcID())
215205
}
@@ -235,16 +225,6 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
235225
case prevDesc.SizeOrBytesIngested() < fd.Info.Size():
236226
e = writeEvent(path, fd, ensureSrcID())
237227
writtenCount++
238-
<<<<<<< HEAD
239-
=======
240-
241-
default:
242-
// For the delete feature we need to run the harvester for
243-
// files that have not changed until they're deleted.
244-
if w.cfg.SendNotChanged {
245-
e = notChangedEvent(path, fd, ensureSrcID())
246-
}
247-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
248228
}
249229

250230
// if none of the conditions were true, the file remained unchanged and we don't need to create an event
@@ -256,14 +236,6 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
256236
}
257237
}
258238

259-
<<<<<<< HEAD
260-
=======
261-
// Record progress metrics for trackable, non-truncated files (tracksHarvesterProgress).
262-
if e.Op != loginp.OpTruncate && tracksHarvesterProgress(&fd, scanOpts) {
263-
harvesterFiles = append(harvesterFiles, loginp.HarvesterFile{ID: ensureSrcID(), Size: fd.Info.Size()})
264-
}
265-
266-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
267239
// delete from previous state to mark that we've seen the existing file again
268240
delete(w.prev, path)
269241
}
@@ -302,14 +274,6 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
302274
case w.events <- createEvent(path, *fd, w.getFileIdentity(*fd)):
303275
createdCount++
304276
}
305-
<<<<<<< HEAD
306-
=======
307-
308-
// New files skip the main loop via early continue, so collect their metrics here.
309-
if tracksHarvesterProgress(fd, scanOpts) {
310-
harvesterFiles = append(harvesterFiles, loginp.HarvesterFile{ID: srcID, Size: fd.Info.Size()})
311-
}
312-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
313277
}
314278

315279
w.log.Debugw("File scan complete",
@@ -324,18 +288,27 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
324288
w.prev = paths
325289
}
326290

327-
<<<<<<< HEAD
328-
=======
329291
// hasClosedHarvesters reports whether any harvester-close notification awaits reconciliation.
292+
// It is a cheap, lock-guarded length check so the watch loop can skip resolving a file identity
293+
// for every scanned file when there is nothing to reconcile (the common steady-state case).
330294
func (w *fileWatcher) hasClosedHarvesters() bool {
331295
w.closedHarvestersMutex.Lock()
332296
defer w.closedHarvestersMutex.Unlock()
333297
return len(w.closedHarvesters) > 0
334298
}
335299

336300
// reconcileClosedHarvester folds a recently-closed harvester's ingested offset (from
337-
// closedHarvesters) into prevDesc so a restarted harvester resumes from the right position.
338-
// It guards a close-during-backoff race that would otherwise withhold writes and lose lines.
301+
// closedHarvesters) into prevDesc so a restarted harvester resumes from the right position,
302+
// then drops the entry.
303+
//
304+
// This prevents a race condition: when the reader/harvester reaches EOF it blocks on a backoff.
305+
// If during this time [logFile.shouldBeClosed] marks the file inactive and closes the reader
306+
// context, once the backoff expires the reader and harvester are closed without ingesting any
307+
// more data. If the fileWatcher sends a write event while the harvester was blocked, no new
308+
// harvester is started because one is already running, yet the fileWatcher updates its internal
309+
// state and won't send further write events until more data is added, so some lines can be
310+
// missed. By applying the offset reported when the harvester closed we realign our state and
311+
// start a new harvester if needed.
339312
func (w *fileWatcher) reconcileClosedHarvester(prevDesc *loginp.FileDescriptor, id string) {
340313
w.closedHarvestersMutex.Lock()
341314
defer w.closedHarvestersMutex.Unlock()
@@ -348,30 +321,6 @@ func (w *fileWatcher) reconcileClosedHarvester(prevDesc *loginp.FileDescriptor,
348321
delete(w.closedHarvesters, id)
349322
}
350323

351-
// tracksHarvesterProgress reports whether a file contributes to the harvester progress metrics.
352-
func tracksHarvesterProgress(fd *loginp.FileDescriptor, opts loginp.FileScanOptions) bool {
353-
return !fd.GZIP && fd.Info.Size() > 0 && !isFileIgnored(*fd, opts)
354-
}
355-
356-
// isFileIgnored returns true when a file is ignored, no matter the reason.
357-
func isFileIgnored(
358-
fd loginp.FileDescriptor,
359-
opts loginp.FileScanOptions,
360-
) bool {
361-
modTime := fd.Info.ModTime()
362-
363-
if opts.IgnoreOlder > 0 && opts.CurrentTime.Sub(modTime) > opts.IgnoreOlder {
364-
return true
365-
}
366-
367-
if !opts.IgnoreInactiveSince.IsZero() && modTime.Sub(opts.IgnoreInactiveSince) <= 0 {
368-
return true
369-
}
370-
371-
return false
372-
}
373-
374-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
375324
// getFileIdentity mimics the same algorithm used by the harvester to generate
376325
// the file identity to any given file.
377326
// See 'startHarvester' on internal/input-logfile/harvester.go.
@@ -404,18 +353,8 @@ func (w *fileWatcher) Event() loginp.FSEvent {
404353
return <-w.events
405354
}
406355

407-
<<<<<<< HEAD
408356
func (w *fileWatcher) GetFiles() map[string]loginp.FileDescriptor {
409357
return w.scanner.GetFiles()
410-
=======
411-
// GetFiles runs a one-off enumeration scan for the prospector's Init and
412-
// TakeOver phases. Unlike the watch loop it does not advance the scanner's
413-
// completedFingerprints set, so these pre-watch scans cannot suppress the
414-
// bridging raw header a still-growing entry needs to migrate its registry key
415-
// after a restart.
416-
func (w *fileWatcher) GetFiles(opts loginp.FileScanOptions) (map[string]loginp.FileDescriptor, loginp.FileScanMetrics) {
417-
return w.scanner.GetFiles(opts)
418-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
419358
}
420359

421360
type fingerprintConfig struct {
@@ -453,22 +392,11 @@ type fileScanner struct {
453392
log *logp.Logger
454393
hasher hash.Hash
455394
readBuffer []byte
456-
<<<<<<< HEAD
457-
=======
458-
compression string
459-
// completedFingerprints holds the paths whose fingerprint was already a
460-
// final SHA-256 on the previous watch-loop scan (growing mode only). The
461-
// bridging raw header is only useful on the scan a file crosses the
462-
// threshold, so for paths in this set toFileDescriptor skips recomputing it.
463-
// GetFiles itself is pure with respect to this set: only fileWatcher.watch
464-
// advances it (after each scan), so the enumeration-only scans the
465-
// prospector runs in Init/TakeOver cannot suppress the header a
466-
// still-growing entry needs to migrate across a restart.
467-
completedFingerprints map[string]struct{}
468395

469396
// lastCount is the number of unique files the previous scan produced.
397+
// It pre-sizes the per-scan maps to avoid repeated grow/rehash allocations
398+
// on a stable file set.
470399
lastCount int
471-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
472400
}
473401

474402
func newFileScanner(logger *logp.Logger, paths []string, config fileScannerConfig) (*fileScanner, error) {
@@ -539,27 +467,13 @@ func (s *fileScanner) normalizeGlobPatterns() error {
539467

540468
// GetFiles returns a map of file descriptors by filenames that
541469
// match the configured paths.
542-
<<<<<<< HEAD
543470
func (s *fileScanner) GetFiles() map[string]loginp.FileDescriptor {
544-
fdByName := map[string]loginp.FileDescriptor{}
545-
=======
546-
func (s *fileScanner) GetFiles(opts loginp.FileScanOptions) (map[string]loginp.FileDescriptor, loginp.FileScanMetrics) {
547-
if opts.CurrentTime.IsZero() {
548-
opts.CurrentTime = time.Now()
549-
}
550-
551-
// Pre-size the per-scan maps from the previous scan's count.
471+
// Pre-size the per-scan maps from the previous scan's unique-file count.
552472
fdByName := make(map[string]loginp.FileDescriptor, s.lastCount)
553-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
554473
// used to determine if a symlink resolves in a already known target
555474
uniqueIDs := make(map[string]string, s.lastCount)
556475
// used to filter out duplicate matches
557-
<<<<<<< HEAD
558-
uniqueFiles := map[string]struct{}{}
559-
=======
560476
uniqueFiles := make(map[string]struct{}, s.lastCount)
561-
scanMetrics := loginp.FileScanMetrics{}
562-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
563477

564478
for _, path := range s.paths {
565479
matches, err := filepath.Glob(path)
@@ -611,13 +525,8 @@ func (s *fileScanner) GetFiles(opts loginp.FileScanOptions) (map[string]loginp.F
611525
}
612526
}
613527

614-
<<<<<<< HEAD
615-
return fdByName
616-
=======
617-
scanMetrics.FilesUnique = int64(len(fdByName))
618528
s.lastCount = len(fdByName)
619-
return fdByName, scanMetrics
620-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
529+
return fdByName
621530
}
622531

623532
type ingestTarget struct {

0 commit comments

Comments
 (0)