Skip to content

Flush remaining data to S3 during shutdown#6424

Merged
dlvenable merged 2 commits intoopensearch-project:mainfrom
Subrahmanyam-Gollapalli:s3-sink-shutdown-data-flush
Feb 5, 2026
Merged

Flush remaining data to S3 during shutdown#6424
dlvenable merged 2 commits intoopensearch-project:mainfrom
Subrahmanyam-Gollapalli:s3-sink-shutdown-data-flush

Conversation

@Subrahmanyam-Gollapalli
Copy link
Copy Markdown
Contributor

Description

Add shutdown method to ensure all remaining buffered data is flushed to S3 before the sink shuts down

Issues Resolved

Prevents data loss when the sink is stopped.

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Subrahmanyam-Gollapalli <subrahmanyam.gollapalli@freshworks.com>
Copy link
Copy Markdown
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Subrahmanyam-Gollapalli for this contribution! I think this approach makes sense to avoid data loss. I have one comment on adding tests here.

* Force flush all remaining S3 groups to S3. This is called during shutdown
* to ensure all buffered data is uploaded before the sink shuts down.
*/
void flushAllRemainingGroups() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit tests on this to ensure that we do correctly flush all the groups.

Copy link
Copy Markdown
Member

@graytaylor0 graytaylor0 Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making this change! The change looks good but we should have some unit tests

Signed-off-by: Subrahmanyam-Gollapalli <subrahmanyam.gollapalli@freshworks.com>
Copy link
Copy Markdown
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you @Subrahmanyam-Gollapalli !

Copy link
Copy Markdown
Member

@graytaylor0 graytaylor0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this change and for adding the tests!

@dlvenable dlvenable merged commit a9cb712 into opensearch-project:main Feb 5, 2026
77 of 80 checks passed
@dlvenable dlvenable added this to the v2.14 milestone Feb 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants