[aws] feat: aws-s3 input registry cleanup for untracked s3 objects by Kavindu-Dodan · Pull Request #41694 · elastic/beats

Kavindu-Dodan · 2024-11-19T21:59:07Z

Proposed commit message

This PR partially addresses #39116 by introducing a registry cleanup strategy for aws-s3 input.

The cleanup implemented here removes registry entries if the s3 object is no longer available (aka tracked) when listing inside the polling lookup. The cleanup removes objects that are not tracked from both the local state and internal store (backed by the registry) to reduce the memory usage.

Note that, this only benefits when s3 objects get removed (ex:- using lifecycle policy) and are no longer available. There should be a follow-up for instances where such removal is not done at the bucket. For example, this could be done by,

Defining a time-based sliding window (ex:- only consider s3 objects of the past 3 days)
Only accept objects of a known storage class (ex:- S3 Standard ) so that users can archive objects ¹

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

Build or run filebeat with aws-s3 input
Add s3 objects and observe memory growth with pprof (regex filter for awss3)
Remove s3 objects and observe memory reduction

Screenshots

Given below are pprof analyss comparisons for ~4K objects in registry and once they were clean up by removing S3 objects (emptying the bucket)

With ~4K objects processed

When S3 objects were removed

Related issues

Relates #39116
Depends on [AWS] improve S3 input states copy by only storing filtered entries #41869

https://docs.aws.amazon.com/AmazonS3/latest/userguide/lifecycle-transition-general-considerations.html ↩

elasticmachine · 2024-11-19T21:59:23Z

Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services)

kaiyan-sheng

Nice work @Kavindu-Dodan ! One quick question first, should we also introduce clean_removed parameter just to be matching the filestream input?

Kavindu-Dodan · 2024-11-20T17:13:53Z

Nice work @Kavindu-Dodan ! One quick question first, should we also introduce clean_removed parameter just to be matching the filestream input?

For s3 input, if clean_removed is set to false, then we will avoid cleanup of registry even for s3 objects that are no longer visible when listing through ListObjectsV2 API. This again can overload the in-memory state store. Besides, there's a core difference between filestream input and s3. In filestream input, we can continuously get new events as file get updated. Whereas in s3, we mark object processed as soon as we process and get ack for all events generated after processing it. So, personally I do not think we need to introduce clean_removed for s3 input unless we see another usage for this. Let me know your view.

x-pack/filebeat/input/awss3/states_test.go

kaiyan-sheng

Looks good to me!

elasticmachine · 2024-11-25T09:20:11Z

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

lucabelluccini · 2024-11-25T12:04:47Z

Looking at the change, if I've correctly understood, we're removing the IDs from the registry once the files have been read.
Shouldn't be removing the entries only when we've acknowledged the shipping of the logs downstream (to Elasticsearch/Outputs)?

Kavindu-Dodan · 2024-11-25T17:30:41Z

Looking at the change, if I've correctly understood, we're removing the IDs from the registry once the files have been read. Shouldn't be removing the entries only when we've acknowledged the shipping of the logs downstream (to Elasticsearch/Outputs)?

The cleanup happens for objects we no longer see in the configured bucket and they won't be processed so there won't be any elastic acknowledgment. Any known bucket entry will be kept in the registry and will be tracked for their success or failure. See this logic [1] which is unchanged.

[1] -

beats/x-pack/filebeat/input/awss3/s3_input.go

Lines 185 to 193 in 9b29500

    
           acks.Add(publishCount, func() { 
        
           	err := in.states.AddState(state) 
        
           	if err != nil { 
        
           		in.log.Errorf("saving completed object state: %v", err.Error()) 
        
           	} 
        
           	// Metrics 
        
           	in.metrics.s3ObjectsAckedTotal.Inc() 
        
           })

x-pack/filebeat/input/awss3/states.go

x-pack/filebeat/input/awss3/s3_input.go

Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> # Conflicts: # x-pack/filebeat/input/awss3/states.go # x-pack/filebeat/input/awss3/states_test.go

Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> # Conflicts: # x-pack/filebeat/input/awss3/states.go # x-pack/filebeat/input/awss3/states_test.go (cherry picked from commit 583d345)

Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> # Conflicts: # x-pack/filebeat/input/awss3/states.go # x-pack/filebeat/input/awss3/states_test.go (cherry picked from commit 583d345) Co-authored-by: Kavindu Dodanduwa <Kavindu-Dodan@users.noreply.github.com>

lucabelluccini · 2025-01-06T10:21:56Z

Hello @Kavindu-Dodan - can we make sure the documentation is also updated with such setting? I do not see any documentation about clean_removed in https://www.elastic.co/guide/en/beats/filebeat/current/filebeat-input-aws-s3.html

Update: I see the concept of clean_removed has been dropped.

Kavindu-Dodan · 2025-01-06T20:50:52Z

@lucabelluccini correct, there's no setting and clean-up occur by default when the object is no longer visible in the listing.

Kavindu-Dodan added enhancement backport-8.x Automated backport to the 8.x branch with mergify labels Nov 19, 2024

Kavindu-Dodan requested a review from a team as a code owner November 19, 2024 21:59

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 19, 2024

Kavindu-Dodan added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label Nov 19, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 19, 2024

mergify bot assigned Kavindu-Dodan Nov 19, 2024

Kavindu-Dodan force-pushed the feat/s3-registry-cleanup branch from c129de6 to f394447 Compare November 19, 2024 21:59

Kavindu-Dodan requested review from faec, kaiyan-sheng and zmoog November 19, 2024 22:40

kaiyan-sheng reviewed Nov 20, 2024

View reviewed changes

Kavindu-Dodan requested a review from kaiyan-sheng November 20, 2024 18:38

Kavindu-Dodan force-pushed the feat/s3-registry-cleanup branch from 42ae2ec to fc03402 Compare November 21, 2024 22:31

kaiyan-sheng reviewed Nov 22, 2024

View reviewed changes

x-pack/filebeat/input/awss3/states_test.go Show resolved Hide resolved

Kavindu-Dodan force-pushed the feat/s3-registry-cleanup branch from fc03402 to 9b29500 Compare November 22, 2024 16:15

kaiyan-sheng approved these changes Nov 22, 2024

View reviewed changes

pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Nov 25, 2024

faec reviewed Nov 25, 2024

View reviewed changes

x-pack/filebeat/input/awss3/states.go Show resolved Hide resolved

Kavindu-Dodan requested a review from faec November 26, 2024 15:33

faec reviewed Nov 26, 2024

View reviewed changes

x-pack/filebeat/input/awss3/s3_input.go Show resolved Hide resolved

Kavindu-Dodan requested a review from faec November 26, 2024 20:56

Kavindu-Dodan mentioned this pull request Nov 26, 2024

[Draft] [aws] [s3] Introduce ignore_older & start_timestamp for s3 input #41804

Closed

6 tasks

Kavindu-Dodan requested review from platform-ingest and removed request for platform-ingest November 27, 2024 14:47

Kavindu-Dodan removed request for a team November 27, 2024 22:48

Kavindu-Dodan mentioned this pull request Dec 3, 2024

[AWS] improve S3 input states copy by only storing filtered entries #41869

Merged

6 tasks

elastic deleted a comment from mergify bot Dec 3, 2024

mergify bot mentioned this pull request Dec 3, 2024

[8.x](backport #41869) [AWS] improve S3 input states copy by only storing filtered entries #41883

Merged

6 tasks

registry cleanup for unavailable objects

3c84e63

Signed-off-by: Kavindu Dodanduwa <kavindu.dodanduwa@elastic.co> # Conflicts: # x-pack/filebeat/input/awss3/states.go # x-pack/filebeat/input/awss3/states_test.go

Kavindu-Dodan force-pushed the feat/s3-registry-cleanup branch from b17324a to 3c84e63 Compare December 3, 2024 23:00

elastic deleted a comment from mergify bot Dec 3, 2024

faec approved these changes Dec 5, 2024

View reviewed changes

This was referenced Dec 5, 2024

[8.16](backport #41869) [AWS] improve S3 input states copy by only storing filtered entries #41921

Merged

[8.17](backport #41869) [AWS] improve S3 input states copy by only storing filtered entries #41922

Merged

Kavindu-Dodan added backport-8.16 Automated backport with mergify backport-8.17 Automated backport with mergify labels Dec 5, 2024

Kavindu-Dodan merged commit 583d345 into elastic:main Dec 6, 2024

Kavindu-Dodan deleted the feat/s3-registry-cleanup branch December 6, 2024 19:56

mergify bot mentioned this pull request Jan 7, 2025

[8.x](backport #41817) [aws] [s3] Introduce ignore_older & start_timestamp for S3 input allowing better registry cleanups #42246

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[aws] feat: aws-s3 input registry cleanup for untracked s3 objects#41694

[aws] feat: aws-s3 input registry cleanup for untracked s3 objects#41694
Kavindu-Dodan merged 1 commit intoelastic:mainfrom
Kavindu-Dodan:feat/s3-registry-cleanup

Kavindu-Dodan commented Nov 19, 2024 •

edited

Loading

Uh oh!

elasticmachine commented Nov 19, 2024

Uh oh!

kaiyan-sheng left a comment

Uh oh!

Kavindu-Dodan commented Nov 20, 2024

Uh oh!

Uh oh!

kaiyan-sheng left a comment

Uh oh!

elasticmachine commented Nov 25, 2024

Uh oh!

lucabelluccini commented Nov 25, 2024

Uh oh!

Kavindu-Dodan commented Nov 25, 2024

Uh oh!

Uh oh!

Uh oh!

lucabelluccini commented Jan 6, 2025 •

edited

Loading

Uh oh!

Kavindu-Dodan commented Jan 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

Kavindu-Dodan commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed commit message

Checklist

How to test this PR locally

Screenshots

Related issues

Footnotes

Uh oh!

elasticmachine commented Nov 19, 2024

Uh oh!

kaiyan-sheng left a comment

Choose a reason for hiding this comment

Uh oh!

Kavindu-Dodan commented Nov 20, 2024

Uh oh!

Uh oh!

kaiyan-sheng left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Nov 25, 2024

Uh oh!

lucabelluccini commented Nov 25, 2024

Uh oh!

Kavindu-Dodan commented Nov 25, 2024

Uh oh!

Uh oh!

Uh oh!

lucabelluccini commented Jan 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kavindu-Dodan commented Jan 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Kavindu-Dodan commented Nov 19, 2024 •

edited

Loading

lucabelluccini commented Jan 6, 2025 •

edited

Loading