Skip to content

[feature](file reader) Merge stream_load_pipe to the new file reader#15035

Merged
morningman merged 5 commits intoapache:masterfrom
BePPPower:newStreamLoad2
Dec 15, 2022
Merged

[feature](file reader) Merge stream_load_pipe to the new file reader#15035
morningman merged 5 commits intoapache:masterfrom
BePPPower:newStreamLoad2

Conversation

@BePPPower
Copy link
Copy Markdown
Contributor

@BePPPower BePPPower commented Dec 13, 2022

Proposed changes

Issue Number: close #15456
Currently, there are two sets of file readers in Doris, this pr rewrites the old stream_load_pipe with the new file reader.

Problem summary

Describe your changes.

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • No

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

_proto_buffered_bytes(0),
_max_buffered_bytes(max_buffered_bytes),
_min_chunk_size(min_chunk_size),
_total_length(total_length),
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.

warning: member initializer '_total_length' does not name a non-static data member or base class [clang-diagnostic-error]

          _total_length(total_length),
          ^

@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@hello-stephen
Copy link
Copy Markdown
Contributor

hello-stephen commented Dec 13, 2022

TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 34.69 seconds
load time: 476 seconds
storage size: 17123356951 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20221214115147_clickbench_pr_63318.html

@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Copy Markdown
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Dec 15, 2022
@github-actions
Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@morningman morningman merged commit c6d93f7 into apache:master Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement](file reader) Refactor file reader

3 participants