Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add folder-based partitioning for s3 scan source #4455

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

graytaylor0
Copy link
Member

@graytaylor0 graytaylor0 commented Apr 24, 2024

Description

This change adds support for the s3 scan source to partition based on folders instead of objects. This means that, unlike object partitions, folder partitions will never be marked as complete, and each node / worker that grabs a folder partition will list and process objects in that folder, then give it up. The use case for this is order guarantee within S3 folders, as well as aggregations within S3 folders, as the same worker / node will process objects from the same S3 folder. To enable this mode, one must set the following configuration.

source:
  s3:
    scan:
      folder_partitions:
        # The depth of the folders to partition on. For example, given object keys "folder-1/folder-3/folder-4/"  and "folder-1/folder-2/folder-5/", a depth of 1 would result in one partition of "folder-1/", while a depth of 2 would result in 2 partitions, "folder-1/folder-3/" and "folder-1/folder-2/". If an object does not contain enough depth, it will not be included in the scan.
        depth: 2 # Defaults to 1
        
        # The max number of objects to process before giving up the folder partition. If the number of objects in the folder is less than this maximum, then all of the objects in the folder will be processed before giving up the partition
        max_objects_per_ownership: 10 # Defaults to 50

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: Taylor Gray <tylgry@amazon.com>
@graytaylor0 graytaylor0 force-pushed the S3FolderPartitioning branch from fd397be to 9b584fa Compare April 24, 2024 19:34
oeyh
oeyh previously approved these changes Apr 25, 2024
@@ -104,6 +104,16 @@ boolean isCodecProvidedWhenNeeded() {
return true;
}

@AssertTrue(message = "acknowledgments and delete_s3_objects_on_read must both be set to true when using PREFIX partition mode")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: we should probably be consistent on the naming. I see both prefix partition and folder partition being used.

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch this was an outdated message where I initially had a mode of PREFIX or FULL_OBJECT_KEY

Signed-off-by: Taylor Gray <tylgry@amazon.com>
oeyh
oeyh previously approved these changes Apr 25, 2024

static final Duration NO_OBJECTS_FOUND_BEFORE_PARTITION_DELETION_DURATION = Duration.ofMinutes(3);
Copy link
Member

@dinujoh dinujoh Apr 25, 2024

Choose a reason for hiding this comment

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

Need to bump this up.

Comment on lines 258 to 260
if (folderPartitionProgressState.isEmpty()) {
folderPartitionProgressState = Optional.of(new S3SourceProgressState(Instant.now().toEpochMilli()));
} else {
Copy link
Member

Choose a reason for hiding this comment

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

This is duplicate. Can be moved to beginning to the method.


AcknowledgementSet acknowledgementSet = null;
String activeAcknowledgmentSetId = null;
while (objectIndex < objectsToProcess.size() && objectsProcessed < folderPartitioningOptions.getMaxObjectsPerOwnership()) {
Copy link
Member

Choose a reason for hiding this comment

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

This method can be split into smaller methods for readability. We can move this while loop to different method.

Signed-off-by: Taylor Gray <tylgry@amazon.com>
@graytaylor0 graytaylor0 merged commit 9b1b05e into opensearch-project:main Apr 26, 2024
47 checks passed
@graytaylor0 graytaylor0 deleted the S3FolderPartitioning branch April 26, 2024 04:46
@kkondaka kkondaka added this to the v2.8 milestone May 14, 2024
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.

4 participants