Skip to content

Implement task payload management with azure storage#15695

Merged
georgew5656 merged 2 commits intoapache:masterfrom
georgew5656:azureTaskPayload
Jan 18, 2024
Merged

Implement task payload management with azure storage#15695
georgew5656 merged 2 commits intoapache:masterfrom
georgew5656:azureTaskPayload

Conversation

@georgew5656
Copy link
Copy Markdown
Contributor

Description

Implement pushTaskPayload/streamTaskPayload for azure blob storage so mmless ingestion can run with larger ingestion payloads when using azure as the deep storage location.

Fixed the bug ...

Renamed the class ...

Added a forbidden-apis entry ...

Currently, if you try running mmless ingestion with azure blob storage as the deep storage provider and use a task payload that is too large, the task logs will throw a NotImplementedException because the relevant functions are not implemented.

Release note

Support ingestion payloads larger than ~1MB for mmless ingestion on azure.

Key changed/added classes in this PR
  • AzureTaskLogs
  • AzureTaskLogsTest

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@Override
public Optional<InputStream> streamTaskPayload(String taskid) throws IOException
{
return streamTaskFile(taskid, 0, getTaskPayloadKey(taskid));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems the taskid is not used in streamTaskFile method thus can be removed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

@georgew5656 georgew5656 requested a review from YongGang January 18, 2024 15:39
Copy link
Copy Markdown
Contributor

@YongGang YongGang left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@suneet-s suneet-s left a comment

Choose a reason for hiding this comment

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

🚢 thanks @georgew5656

@georgew5656 georgew5656 merged commit dd03f70 into apache:master Jan 18, 2024
@adarshsanjeev adarshsanjeev added this to the 30.0.0 milestone May 6, 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