Add change to s3 scan source to create partitions after each page of … by Davidding4718 · Pull Request #6006 · opensearch-project/data-prepper

Davidding4718 · 2025-08-21T22:29:34Z

(cherry picked from commit bf74cb7)

Description

Implements global state ownership renewal functionality to prevent ownership timeouts during long-running S3 partition creation operations:

This change adds a renewGlobalStatePartitionOwnership() method to LeaseBasedSourceCoordinator and integrates periodic ownership renewal (every 2 minutes) in S3ScanPartitionCreationSupplier to maintain ownership during large S3 bucket scanning operations.
The implementation ensures reliable partition creation for large S3 buckets without losing global state ownership due to lease expiration.

Issues Resolved

Resolves #5039

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.

…objects listed Signed-off-by: Taylor Gray <tylgry@amazon.com> (cherry picked from commit bf74cb7)

Signed-off-by: Siqi Ding <dingdd@amazon.com>

graytaylor0 · 2025-08-27T18:52:02Z


-    private void createPartitions(final List<PartitionIdentifier> partitionIdentifiers) {
+    @Override
+    public void createPartitions(final List<PartitionIdentifier> partitionIdentifiers) {


Is the work to renew global partition ownership left out of this PR?

graytaylor0 · 2025-08-27T18:52:22Z

-                    .collect(Collectors.toList()));
-
+                    .collect(Collectors.toList());
            LOG.info("Found page of {} objects from bucket {}", listObjectsV2Response.keyCount(), bucket);


We should have a check to update global state ownership here

graytaylor0 · 2025-08-27T18:53:27Z

+                sourceCoordinator.createPartitions(folderPartitionsForPage);
+            } else {
+                LOG.info("Creating partitions for {} S3 objects from bucket {}", partitionsForPage.size(), bucket);
+                sourceCoordinator.createPartitions(partitionsForPage);


It is worth considering adding some way to update global state ownership periodically in this call to create partitions as this can potentially take a long time. I would probably have the LeaseBasedSourceCoordinator do this within this method.

Have added a method to update global state ownership periodically

Signed-off-by: Siqi Ding <dingdd@amazon.com>

graytaylor0 · 2025-09-02T20:52:57Z

        return globalStateItemForPartitionCreation;
    }

+    private void renewGlobalStateOwnershipForPartitionCreation() {


We should be updating this ownership during the scan as well

Have made the change and added the update during the scan.

Prevents timeout during long-running operations by renewing global state ownership every 3 minutes Signed-off-by: Siqi Ding <dingdd@amazon.com>

graytaylor0 · 2025-09-09T16:33:11Z

+            if (Duration.between(lastOwnershipRenewal, Instant.now()).compareTo(OWNERSHIP_RENEWAL_INTERVAL) >= 0) {
+                sourceCoordinator.renewGlobalStateOwnershipForPartitionCreation();
+                lastOwnershipRenewal = Instant.now();
+            }


You should remove all the we don't need to update global state in the ScanObjectWorker

Got it. Will remove them.

graytaylor0 · 2025-09-09T16:33:59Z

+            if (Duration.between(lastOwnershipRenewal, Instant.now()).compareTo(OWNERSHIP_RENEWAL_INTERVAL) >= 0) {
+                sourceCoordinator.renewGlobalStateOwnershipForPartitionCreation();
+                lastOwnershipRenewal = Instant.now();
+            }


Same here we global state is already done so shouldn't be renewing it

Will remove them.

graytaylor0 · 2025-09-09T16:38:48Z

+        Instant lastOwnershipRenewal = Instant.now();
+
        for (final PartitionIdentifier partitionIdentifier : partitionIdentifiers) {
+            if (Instant.now().isAfter(lastOwnershipRenewal.plus(OWNERSHIP_RENEWAL_INTERVAL))) {


We should handle the case where there are 0 partitions getting passed here but the bucket has many objects to scan. I would make this lastGlobalOwnershipRenewal a variable of the class, then check it one time before you go into the loop of partitions. Break this snippet of code out into a private method

Have made changed based on this comment. Thank you!

…nershipRenewal related renewGlobalOwnershipIfNeeded to ensures ownership is maintained both before starting and throughout the entire partition creation process Signed-off-by: Siqi Ding <dingdd@amazon.com>

graytaylor0 · 2025-09-09T21:24:28Z

    }

+    @Override
+    public void renewGlobalStateOwnershipForPartitionCreation() {


I think this can actually doesn't have to be public right? Probably ok as is though

Have changed to private. Thank you!

Signed-off-by: Siqi Ding <dingdd@amazon.com>

Add change to s3 scan source to create partitions after each page of …

107e594

…objects listed Signed-off-by: Taylor Gray <tylgry@amazon.com> (cherry picked from commit bf74cb7)

Davidding4718 requested review from KarstenSchnitter, chenqi0805, dinujoh, dlvenable, engechas, graytaylor0, kkondaka, oeyh, san81, sb2k16 and srikanthjg as code owners August 21, 2025 22:29

Renew the unit test to handle the two scans and seperate buckets

359b7d0

Signed-off-by: Siqi Ding <dingdd@amazon.com>

Davidding4718 force-pushed the S3ScanChange branch from e08270a to 359b7d0 Compare August 22, 2025 15:36

graytaylor0 reviewed Aug 27, 2025

View reviewed changes

Add global state ownership renewal during partition creation

550fc20

Signed-off-by: Siqi Ding <dingdd@amazon.com>

Davidding4718 force-pushed the S3ScanChange branch from 543cd61 to 550fc20 Compare September 2, 2025 20:10

graytaylor0 reviewed Sep 2, 2025

View reviewed changes

Add ownership renewal during S3 partition creation

e535221

Prevents timeout during long-running operations by renewing global state ownership every 3 minutes Signed-off-by: Siqi Ding <dingdd@amazon.com>

graytaylor0 reviewed Sep 9, 2025

View reviewed changes

graytaylor0 previously approved these changes Sep 9, 2025

View reviewed changes

Remove the each time update in ScanObjectWorker; Add the lastGlobalOw…

793f5d3

…nershipRenewal related renewGlobalOwnershipIfNeeded to ensures ownership is maintained both before starting and throughout the entire partition creation process Signed-off-by: Siqi Ding <dingdd@amazon.com>

Davidding4718 force-pushed the S3ScanChange branch from 384e649 to 793f5d3 Compare September 9, 2025 20:56

graytaylor0 reviewed Sep 9, 2025

View reviewed changes

Make renewGlobalStateOwnershipForPartitionCreation private

0f402ed

Signed-off-by: Siqi Ding <dingdd@amazon.com>

Davidding4718 dismissed graytaylor0’s stale review via 0f402ed September 9, 2025 22:04

graytaylor0 approved these changes Sep 9, 2025

View reviewed changes

oeyh approved these changes Sep 9, 2025

View reviewed changes

oeyh merged commit 771feac into opensearch-project:main Sep 10, 2025
46 of 47 checks passed

Conversation

Davidding4718 commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Davidding4718 commented Aug 21, 2025 •

edited

Loading