Refactor Thrift Transport for Parquet Metadata Access #4160
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for meta-velox canceled.
|
facebook-github-bot
added
the
CLA Signed
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
5 times, most recently
from
43480e7 to
3373687
Compare
liushengxuan
changed the title
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
from
3373687 to
0758a1d
Compare
Yuhta
reviewed
Mar 1, 2023
Yuhta
reviewed
Mar 1, 2023
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
3 times, most recently
from
c59787d to
b18a681
Compare
Yuhta
reviewed
Mar 1, 2023
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
from
b18a681 to
531f3b4
Compare
leoluan2009
reviewed
Mar 2, 2023
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
from
531f3b4 to
9471f36
Compare
yingsu00
requested changes
Mar 2, 2023
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
2 times, most recently
from
79f7029 to
d9560c6
Compare
liushengxuan
requested review from
leoluan2009,
yingsu00 and
Yuhta
and removed request for
leoluan2009,
yingsu00 and
Yuhta
liushengxuan
requested review from
Yuhta,
yingsu00 and
leoluan2009
and removed request for
yingsu00 and
Yuhta
yingsu00
requested changes
Mar 3, 2023
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
3 times, most recently
from
ad03558 to
4ef5b02
Compare
yingsu00
reviewed
Mar 3, 2023
There was a problem hiding this comment.
@liushengxuan Will you be able to update the footer reading using the new transport as well? If yes we don't need to keep the old transport.
Yuhta
reviewed
Mar 3, 2023
|
@Yuhta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Yuhta
reviewed
Mar 3, 2023
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
from
4ef5b02 to
6204515
Compare
This PR refactors the Thrift Transport for Parquet Metadata access. It uses ThriftTransport as an interface and introduces ThriftBufferedTransport and ThriftStreamingTransport. ThriftStreamingTransport takes in a SeekableInputStream as input for Thrift parsing. This can be used for Parquet Page Header parsing. This optimization is able to reduce the deep copy in readPageHeader(). And it is also the prerequisite to fix the incorrect page header length issue. ThriftBufferedTransport takes in a consecutive memory space as input for Thrift parsing. This can be used for Parquet Footer parsing, because the footer is at the bottom of the file and we need to also take care of footer length and PAR1, from the bottom to top.
liushengxuan
force-pushed
the
shengxuan/refactor_thrift_transport
branch
from
6204515 to
df4a80c
Compare
|
@Yuhta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
PHILO-HE
added a commit
to PHILO-HE/velox
that referenced
this pull request
Mar 7, 2023
zhejiangxiaomai
pushed a commit
to oap-project/velox
that referenced
this pull request
Mar 8, 2023
…g data (#145) * Port a patch: Refactor Thrift Transport for Parquet Metadata Access facebookincubator#4160 * Port a patch: Read Parquet Page Header with ThriftStreamingTransport to Fix the Incorrect Header Length facebookincubator#4108
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Mar 8, 2023
…g data (oap-project#145) * Port a patch: Refactor Thrift Transport for Parquet Metadata Access facebookincubator#4160 * Port a patch: Read Parquet Page Header with ThriftStreamingTransport to Fix the Incorrect Header Length facebookincubator#4108
zhejiangxiaomai
pushed a commit
to zhejiangxiaomai/velox
that referenced
this pull request
Mar 8, 2023
…g data (oap-project#145) * Port a patch: Refactor Thrift Transport for Parquet Metadata Access facebookincubator#4160 * Port a patch: Read Parquet Page Header with ThriftStreamingTransport to Fix the Incorrect Header Length facebookincubator#4108
PHILO-HE
added a commit
to PHILO-HE/velox
that referenced
this pull request
Mar 27, 2023
…okincubator#4160)" This reverts commit 39977b1.
PHILO-HE
added a commit
to PHILO-HE/velox
that referenced
this pull request
Mar 27, 2023
…okincubator#4160)" This reverts commit 39977b1.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the Thrift Transport for Parquet Metadata access. It uses
ThriftTransportas an interface and introducesThriftBufferedTransportandThriftStreamingTransport.ThriftStreamingTransporttakes in aSeekableInputStreamas input for Thrift parsing. This can be used for Parquet Page Header parsing. This optimization is able to reduce the deep copy inreadPageHeader(). And it is also the prerequisite to fix the incorrect page header length issue.ThriftBufferedTransporttakes in a consecutive memory space as input for Thrift parsing. This can be used for Parquet Footer parsing, because the footer is at the bottom of the file and we need to also take care offooter lengthandPAR1, from the bottom to top.