Skip to content

Commit 9c81038

Browse files
committed
Resolve Mergify backport conflicts on 9.3
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 9.3. 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 7ce32d4 commit 9c81038

2 files changed

Lines changed: 63 additions & 211 deletions

File tree

filebeat/input/filestream/fswatch.go

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

175-
<<<<<<< HEAD
176175
paths := w.scanner.GetFiles()
177-
=======
178-
// file identity is updated in GetFiles
179-
now := time.Now()
180-
scanOpts := loginp.FileScanOptions{
181-
CurrentTime: now,
182-
IgnoreOlder: ignoreOlder,
183-
IgnoreInactiveSince: ignoreInactiveSince,
184-
}
185-
paths, scanMetrics := w.scanner.GetFiles(scanOpts)
186-
metrics.UpdateFileScanMetrics(scanMetrics)
187-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
188176

189177
// for debugging purposes
190178
writtenCount := 0
@@ -207,8 +195,9 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
207195
continue
208196
}
209197

210-
// srcID is the file identity (harvester ID/registry key), resolved lazily via ensureSrcID:
211-
// an unchanged, untracked file (gzip, empty, ignore_older) never needs one, saving allocs.
198+
// srcID is the file identity (harvester ID/registry key), resolved lazily via
199+
// ensureSrcID: an unchanged file that produces no event never needs one, saving a
200+
// per-file identity allocation on every idle scan.
212201
var srcID string
213202
ensureSrcID := func() string {
214203
if srcID == "" { // getFileIdentity never returns ""
@@ -217,7 +206,8 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
217206
return srcID
218207
}
219208

220-
// closedHarvesters is empty in the steady state; this reconciliation is usually skipped.
209+
// closedHarvesters is empty in the steady state, so this reconciliation is usually
210+
// skipped and the file identity is never resolved.
221211
if w.hasClosedHarvesters() {
222212
w.reconcileClosedHarvester(&prevDesc, ensureSrcID())
223213
}
@@ -260,14 +250,6 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
260250
}
261251
}
262252

263-
<<<<<<< HEAD
264-
=======
265-
// Record progress metrics for trackable, non-truncated files (tracksHarvesterProgress).
266-
if e.Op != loginp.OpTruncate && tracksHarvesterProgress(&fd, scanOpts) {
267-
harvesterFiles = append(harvesterFiles, loginp.HarvesterFile{ID: ensureSrcID(), Size: fd.Info.Size()})
268-
}
269-
270-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
271253
// delete from previous state to mark that we've seen the existing file again
272254
delete(w.prev, path)
273255
}
@@ -307,14 +289,6 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
307289
case w.events <- createEvent(path, *fd, w.getFileIdentity(*fd)):
308290
createdCount++
309291
}
310-
<<<<<<< HEAD
311-
=======
312-
313-
// New files skip the main loop via early continue, so collect their metrics here.
314-
if tracksHarvesterProgress(fd, scanOpts) {
315-
harvesterFiles = append(harvesterFiles, loginp.HarvesterFile{ID: srcID, Size: fd.Info.Size()})
316-
}
317-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
318292
}
319293

320294
w.log.Debugw("File scan complete",
@@ -329,18 +303,27 @@ func (w *fileWatcher) watch(ctx unison.Canceler) {
329303
w.prev = paths
330304
}
331305

332-
<<<<<<< HEAD
333-
=======
334306
// hasClosedHarvesters reports whether any harvester-close notification awaits reconciliation.
307+
// It is a cheap, lock-guarded length check so the watch loop can skip resolving a file identity
308+
// for every scanned file when there is nothing to reconcile (the common steady-state case).
335309
func (w *fileWatcher) hasClosedHarvesters() bool {
336310
w.closedHarvestersMutex.Lock()
337311
defer w.closedHarvestersMutex.Unlock()
338312
return len(w.closedHarvesters) > 0
339313
}
340314

341315
// reconcileClosedHarvester folds a recently-closed harvester's ingested offset (from
342-
// closedHarvesters) into prevDesc so a restarted harvester resumes from the right position.
343-
// It guards a close-during-backoff race that would otherwise withhold writes and lose lines.
316+
// closedHarvesters) into prevDesc so a restarted harvester resumes from the right position,
317+
// then drops the entry.
318+
//
319+
// This prevents a race condition: when the reader/harvester reaches EOF it blocks on a backoff.
320+
// If during this time [logFile.shouldBeClosed] marks the file inactive and closes the reader
321+
// context, once the backoff expires the reader and harvester are closed without ingesting any
322+
// more data. If the fileWatcher sends a write event while the harvester was blocked, no new
323+
// harvester is started because one is already running, yet the fileWatcher updates its internal
324+
// state and won't send further write events until more data is added, so some lines can be
325+
// missed. By applying the offset reported when the harvester closed we realign our state and
326+
// start a new harvester if needed.
344327
func (w *fileWatcher) reconcileClosedHarvester(prevDesc *loginp.FileDescriptor, id string) {
345328
w.closedHarvestersMutex.Lock()
346329
defer w.closedHarvestersMutex.Unlock()
@@ -353,30 +336,6 @@ func (w *fileWatcher) reconcileClosedHarvester(prevDesc *loginp.FileDescriptor,
353336
delete(w.closedHarvesters, id)
354337
}
355338

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

416-
<<<<<<< HEAD
417375
func (w *fileWatcher) GetFiles() map[string]loginp.FileDescriptor {
418376
return w.scanner.GetFiles()
419-
=======
420-
// GetFiles runs a one-off enumeration scan for the prospector's Init and
421-
// TakeOver phases. Unlike the watch loop it does not advance the scanner's
422-
// completedFingerprints set, so these pre-watch scans cannot suppress the
423-
// bridging raw header a still-growing entry needs to migrate its registry key
424-
// after a restart.
425-
func (w *fileWatcher) GetFiles(opts loginp.FileScanOptions) (map[string]loginp.FileDescriptor, loginp.FileScanMetrics) {
426-
return w.scanner.GetFiles(opts)
427-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
428377
}
429378

430379
type fingerprintConfig struct {
@@ -463,21 +412,11 @@ type fileScanner struct {
463412
hasher hash.Hash
464413
readBuffer []byte
465414
compression string
466-
<<<<<<< HEAD
467-
=======
468-
// completedFingerprints holds the paths whose fingerprint was already a
469-
// final SHA-256 on the previous watch-loop scan (growing mode only). The
470-
// bridging raw header is only useful on the scan a file crosses the
471-
// threshold, so for paths in this set toFileDescriptor skips recomputing it.
472-
// GetFiles itself is pure with respect to this set: only fileWatcher.watch
473-
// advances it (after each scan), so the enumeration-only scans the
474-
// prospector runs in Init/TakeOver cannot suppress the header a
475-
// still-growing entry needs to migrate across a restart.
476-
completedFingerprints map[string]struct{}
477415

478416
// lastCount is the number of unique files the previous scan produced.
417+
// It pre-sizes the per-scan maps to avoid repeated grow/rehash allocations
418+
// on a stable file set.
479419
lastCount int
480-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
481420
}
482421

483422
func newFileScanner(logger *logp.Logger, paths []string, config fileScannerConfig, compression string) (*fileScanner, error) {
@@ -549,27 +488,13 @@ func (s *fileScanner) normalizeGlobPatterns() error {
549488

550489
// GetFiles returns a map of file descriptors by filenames that
551490
// match the configured paths.
552-
<<<<<<< HEAD
553491
func (s *fileScanner) GetFiles() map[string]loginp.FileDescriptor {
554-
fdByName := map[string]loginp.FileDescriptor{}
555-
=======
556-
func (s *fileScanner) GetFiles(opts loginp.FileScanOptions) (map[string]loginp.FileDescriptor, loginp.FileScanMetrics) {
557-
if opts.CurrentTime.IsZero() {
558-
opts.CurrentTime = time.Now()
559-
}
560-
561-
// Pre-size the per-scan maps from the previous scan's count.
492+
// Pre-size the per-scan maps from the previous scan's unique-file count.
562493
fdByName := make(map[string]loginp.FileDescriptor, s.lastCount)
563-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
564494
// used to determine if a symlink resolves in a already known target
565495
uniqueIDs := make(map[string]string, s.lastCount)
566496
// used to filter out duplicate matches
567-
<<<<<<< HEAD
568-
uniqueFiles := map[string]struct{}{}
569-
=======
570497
uniqueFiles := make(map[string]struct{}, s.lastCount)
571-
scanMetrics := loginp.FileScanMetrics{}
572-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
573498

574499
for _, path := range s.paths {
575500
matches, err := filepath.Glob(path)
@@ -621,13 +546,8 @@ func (s *fileScanner) GetFiles(opts loginp.FileScanOptions) (map[string]loginp.F
621546
}
622547
}
623548

624-
<<<<<<< HEAD
625-
return fdByName
626-
=======
627-
scanMetrics.FilesUnique = int64(len(fdByName))
628549
s.lastCount = len(fdByName)
629-
return fdByName, scanMetrics
630-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
550+
return fdByName
631551
}
632552

633553
type ingestTarget struct {

filebeat/input/filestream/fswatch_test.go

Lines changed: 41 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,64 +1139,11 @@ func BenchmarkGetFilesWithFingerprint(b *testing.B) {
11391139
}
11401140
}
11411141

1142-
<<<<<<< HEAD
1143-
=======
1144-
// BenchmarkGetFilesWithFingerprintGrowing measures repeated scans of stable,
1145-
// above-threshold files. Each GetFiles call is one scan, so b.N iterations model
1146-
// files that have grown past the fingerprint threshold and then sit unchanged
1147-
// across many scans — the scenario the sub-threshold growing benchmarks in
1148-
// BenchmarkFilestream do not exercise.
1149-
//
1150-
// The "static" and "growing" sub-benchmarks fingerprint the identical
1151-
// above-threshold files; their only difference is growing mode. The delta is the
1152-
// per-scan raw-header re-encode that growing mode performs on every completed
1153-
// file to bridge a possible threshold crossing. It calls GetFiles directly
1154-
// without advancing the completedFingerprints set, so it models the UNSUPPRESSED
1155-
// cost — exactly the work the watch loop elides on stable files by tracking
1156-
// completed paths. It therefore quantifies the per-scan cost that suppression
1157-
// removes after a file's first crossing scan.
1158-
func BenchmarkGetFilesWithFingerprintGrowing(b *testing.B) {
1159-
cases := []struct {
1160-
name string
1161-
growing bool
1162-
}{
1163-
{"static", false},
1164-
{"growing", true},
1165-
}
1166-
for _, tc := range cases {
1167-
b.Run(tc.name, func(b *testing.B) {
1168-
paths := writeBenchmarkFiles(b, b.TempDir(), benchmarkFileCount)
1169-
cfg := fileScannerConfig{
1170-
Fingerprint: fingerprintConfig{
1171-
Enabled: true,
1172-
Offset: 0,
1173-
Length: 1024,
1174-
Growing: tc.growing,
1175-
},
1176-
}
1177-
s, err := newFileScanner(logp.NewNopLogger(), paths, cfg, CompressionNone)
1178-
require.NoError(b, err)
1179-
1180-
b.ResetTimer()
1181-
for i := 0; i < b.N; i++ {
1182-
files, _ := s.GetFiles(loginp.FileScanOptions{})
1183-
require.Len(b, files, benchmarkFileCount)
1184-
}
1185-
})
1186-
}
1187-
}
1188-
1189-
// BenchmarkWatchIdle measures fileWatcher.watch over an unchanged file set: the mostly-idle steady
1190-
// state where allocs/op is the cost of observing nothing changed (identity cache + pre-sized maps).
1142+
// BenchmarkWatchIdle measures fileWatcher.watch over an unchanged file set: the mostly-idle
1143+
// steady state where allocs/op is the cost of observing that nothing changed. With the lazy
1144+
// file-identity resolution and pre-sized scan maps, an idle rescan should not allocate a file
1145+
// identity per file.
11911146
func BenchmarkWatchIdle(b *testing.B) {
1192-
states := []struct {
1193-
name string
1194-
ignoreOlder time.Duration
1195-
aged bool
1196-
}{
1197-
{"active", 0, false},
1198-
{"ignored", time.Hour, true},
1199-
}
12001147
identities := []struct {
12011148
name string
12021149
fingerprint bool
@@ -1209,66 +1156,51 @@ func BenchmarkWatchIdle(b *testing.B) {
12091156
}},
12101157
}
12111158

1212-
for _, st := range states {
1213-
for _, id := range identities {
1214-
for _, fileCount := range []int{100, 1000, 10000} {
1215-
b.Run(fmt.Sprintf("%s/%s/%d_files", st.name, id.name, fileCount), func(b *testing.B) {
1216-
paths := writeBenchmarkFiles(b, b.TempDir(), fileCount)
1217-
1218-
if st.aged {
1219-
// Age every file well past ignoreOlder so it is excluded.
1220-
old := time.Now().Add(-48 * time.Hour)
1221-
matches, err := filepath.Glob(paths[0])
1222-
require.NoError(b, err)
1223-
for _, m := range matches {
1224-
require.NoError(b, os.Chtimes(m, old, old))
1159+
for _, id := range identities {
1160+
for _, fileCount := range []int{100, 1000, 10000} {
1161+
b.Run(fmt.Sprintf("%s/%d_files", id.name, fileCount), func(b *testing.B) {
1162+
paths := writeBenchmarkFiles(b, b.TempDir(), fileCount)
1163+
1164+
cfg := defaultFileWatcherConfig()
1165+
cfg.Scanner.Fingerprint.Enabled = id.fingerprint
1166+
1167+
fw, err := newFileWatcher(
1168+
logp.NewNopLogger(),
1169+
paths,
1170+
cfg,
1171+
CompressionNone,
1172+
false,
1173+
id.newID(),
1174+
mustSourceIdentifier("bench-id"),
1175+
)
1176+
require.NoError(b, err)
1177+
1178+
// A create event is emitted for every file on the first scan; idle rescans emit
1179+
// nothing. Drain in the background so watch's sends never block.
1180+
ctx := b.Context()
1181+
go func() {
1182+
for {
1183+
select {
1184+
case <-ctx.Done():
1185+
return
1186+
case <-fw.events:
12251187
}
12261188
}
1189+
}()
12271190

1228-
cfg := defaultFileWatcherConfig()
1229-
cfg.Scanner.Fingerprint.Enabled = id.fingerprint
1230-
cfg.Scanner.Fingerprint.Growing = id.fingerprint
1231-
1232-
fw, err := newFileWatcher(
1233-
logp.NewNopLogger(),
1234-
paths,
1235-
cfg,
1236-
CompressionNone,
1237-
false,
1238-
id.newID(),
1239-
mustSourceIdentifier("bench-id"),
1240-
)
1241-
require.NoError(b, err)
1242-
1243-
// A create event is emitted for every file on the first scan; idle rescans emit
1244-
// nothing. Drain in the background so watch's sends never block.
1245-
ctx := b.Context()
1246-
go func() {
1247-
for {
1248-
select {
1249-
case <-ctx.Done():
1250-
return
1251-
case <-fw.events:
1252-
}
1253-
}
1254-
}()
1255-
1256-
metrics := newTestMetrics()
1257-
// Prime w.prev so the benchmarked scans are pure steady state.
1258-
fw.watch(ctx, metrics, st.ignoreOlder, time.Time{})
1191+
// Prime w.prev so the benchmarked scans are pure steady state.
1192+
fw.watch(ctx)
12591193

1260-
b.ReportAllocs()
1261-
b.ResetTimer()
1262-
for b.Loop() {
1263-
fw.watch(ctx, metrics, st.ignoreOlder, time.Time{})
1264-
}
1265-
})
1266-
}
1194+
b.ReportAllocs()
1195+
b.ResetTimer()
1196+
for b.Loop() {
1197+
fw.watch(ctx)
1198+
}
1199+
})
12671200
}
12681201
}
12691202
}
12701203

1271-
>>>>>>> c05890c43 (filestream: reduce per-scan allocations (#51863))
12721204
func createWatcherWithConfig(t *testing.T, logger *logp.Logger, paths []string, cfgStr string) *fileWatcher {
12731205
tmpCfg := struct {
12741206
Scaner fileWatcherConfig `config:"scanner"`

0 commit comments

Comments
 (0)