Skip to content

GH-36828: [C++][Parquet] Make buffered RowGroupSerializer using BufferedPageWriter#36829

Merged
wgtmac merged 1 commit intoapache:mainfrom
mapleFU:parquet/fix-unbuffered-writer
Jul 24, 2023
Merged

GH-36828: [C++][Parquet] Make buffered RowGroupSerializer using BufferedPageWriter#36829
wgtmac merged 1 commit intoapache:mainfrom
mapleFU:parquet/fix-unbuffered-writer

Conversation

@mapleFU
Copy link
Copy Markdown
Member

@mapleFU mapleFU commented Jul 23, 2023 •
edited by github-actions bot
Loading

Rationale for this change

See #36828

What changes are included in this PR?

Add buffered argument when building PageWriter

Are these changes tested?

no

Are there any user-facing changes?

no

@mapleFU mapleFU requested a review from wgtmac as a code owner July 23, 2023 10:51
@mapleFU
Copy link
Copy Markdown
Member Author

mapleFU commented Jul 23, 2023

@wgtmac @pitrou
Also cc @yyang52

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 23, 2023

⚠️ GitHub issue #36828 has been automatically assigned in GitHub to PR creator.

wgtmac
wgtmac approved these changes Jul 23, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@wgtmac wgtmac left a comment

Choose a reason for hiding this comment

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

+1

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jul 23, 2023
@yyang52
Copy link
Copy Markdown
Contributor

yyang52 commented Jul 24, 2023

LGTM, thanks for the fixing!

@raulcd
Copy link
Copy Markdown
Member

raulcd commented Jul 24, 2023

@pitrou this doesn't seem to be a blocker for 13.0.0 , right?

@wgtmac
Copy link
Copy Markdown
Member

wgtmac commented Jul 24, 2023

@pitrou this doesn't seem to be a blocker for 13.0.0 , right?

You're right. That commit is not included in the release branch.

@pitrou
Copy link
Copy Markdown
Member

pitrou commented Jul 24, 2023

@wgtmac Do you want to merge?

@wgtmac wgtmac merged commit 78bbe76 into apache:main Jul 24, 2023
@wgtmac wgtmac removed the awaiting committer review Awaiting committer review label Jul 24, 2023
@mapleFU mapleFU deleted the parquet/fix-unbuffered-writer branch July 25, 2023 04:39
@conbench-apache-arrow
Copy link
Copy Markdown

conbench-apache-arrow bot commented Jul 30, 2023

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 78bbe76.

There were 4 benchmark results indicating a performance regression:

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
@mapleFU @loicalleyne
apacheGH-36828: [C++][Parquet] Make buffered RowGroupSerializer using…
36559bf 
… BufferedPageWriter (apache#36829)

### Rationale for this change

See apache#36828

### What changes are included in this PR?

Add `buffered` argument when building `PageWriter`

### Are these changes tested?

no

### Are there any user-facing changes?

no

* Closes: apache#36828

Authored-by: mwish <maplewish117@gmail.com>
Signed-off-by: Gang Wu <ustcwg@gmail.com>
@mapleFU mapleFU mentioned this pull request Mar 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wgtmac wgtmac wgtmac approved these changes

Assignees

No one assigned

Labels

Component: C++ Component: Parquet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[C++][Parquet] Parquet: buffered column writer uses unbuffered page writer

5 participants

@mapleFU @yyang52 @raulcd @wgtmac @pitrou