You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2024/11/26 12:24:09 UTC

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner

Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22123


Change subject: WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner
......................................................................

WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner

Currently only works on MT_DOP>0, i.e. HdfsScanNodeMt.
ScratchBatch MemPools and data page pools are now backed by BufferPool
to reduce tcmalloc contention in the central free list.

TODO: test ORC scanner
TODO: Doesn't work on MT_DOP=0 maybe due to
HdfsScanNode::ReturnReservationFromScannerThread() returns too many
reservation. Reservation used in the output RowBatch needs to be
correctly accounted.

Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
---
M be/src/exec/exec-node.h
M be/src/exec/hdfs-columnar-scanner.cc
M be/src/exec/hdfs-columnar-scanner.h
M be/src/exec/hdfs-scan-node.cc
M be/src/exec/hdfs-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.h
M be/src/exec/parquet/parquet-column-chunk-reader.cc
M be/src/exec/scratch-tuple-batch.h
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
17 files changed, 188 insertions(+), 58 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/22123/1
-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22123 )

Change subject: WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/18455/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 04 Mar 2025 07:41:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/22123

to look at the new patch set (#2).

Change subject: WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner
......................................................................

WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner

ScratchBatch MemPools and data page pools are now backed by BufferPool
to reduce tcmalloc contention in the central free list.

Limitation:
Not change MT_DOP=0, i.e. HdfsScanNode, since
HdfsScanNode::ReturnReservationFromScannerThread() can't return the
reservation requested for the scanner thread. Reservation that
previously used by IO buffers might be used in the output RowBatch so
can't be returned. No such problem in MT_DOP>0 where
HdfsScanNode::ReturnReservationFromScannerThread() is not used.

TODO: calculate the min reservation based on the MemPools requirement.
TODO: test ORC scanner

Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
---
M be/src/exec/exec-node.h
M be/src/exec/hdfs-columnar-scanner.cc
M be/src/exec/hdfs-columnar-scanner.h
M be/src/exec/hdfs-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.h
M be/src/exec/parquet/parquet-column-chunk-reader.cc
M be/src/exec/scratch-tuple-batch.h
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
16 files changed, 211 insertions(+), 56 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/22123/2
-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22123 )

Change subject: WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/19641/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 28 Jun 2025 15:28:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/22123

to look at the new patch set (#5).

Change subject: WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner
......................................................................

WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

MemPool is used as an Arena that pre-allocates memory chunks to save
frequent calls on malloc() and free() from the thread. It's helpful
especially for small objects that have a similar lifetime and fit into
the pre-allocated chunks. However, when allocating a large memory space
that no free chunks can fit, it still invokes malloc() to allocate the
space.

TCMalloc is used in Impala as the memory allocator. It has a ThreadCache
for each thread to serve small allocations. However, for allocations
larger than 256KB, they are served by the Central Cache of TCMalloc
which has lock contention on its CentralFreeList.

Currently, when scanner threads allocate large memory spaces from
MemPool, they suffer from this TCMalloc contention. Performance degrades
as concurrency increases.

BufferPool is another memory management layer in Impala to handle memory
reservation and spill-to-disk for all queries. It also enables reuse of
buffers (memory spaces) between queries, to avoid frequent allocations.
Allocating large buffers from BufferPool won't hit the above TCMalloc
contention issue if the allocations can be served by previously freed
buffers.

This patch extends MemPool to be able to allocate memory chunks using
BufferPool. A new constructor passing a BufferPool client is added to
enable this mode. MemPool now manages a list of memory chunks allocated
from malloc() and a list of buffers allocated from BufferPool. MemPools
in different modes can acquire data from each other, by updating these
two lists.

ScratchTupleBatch is used in Parquet/ORC scanners to materialize tuples
before evaluating filters. It currently has two MemPools, tuple_mem_pool
and aux_mem_pool, for the fix-sized and var-len parts respectively.
ParquetColumnChunkReader has a data_page_pool_ which allocates memory
for decompressed/copied Parquet data pages. These three kinds of
MemPools are where a scanner could allocate large memory space. They are
now backed by BufferPool to avoid the above TCMalloc contention issue.

Min reservation of the HdfsScanNode is increased for these allocation.

----------
Limitation

When allocating memory from BufferPool, MemPool uses
AllocateUnreservedBuffer() which might increase memory reservation in
runtime to fit the space that used to be allocated from malloc(). This
makes the query easier to hit OOM when it couldn't increase the
reservation. Some tests that expect the query can run with the minimal
reservation, i.e.
DEBUG_ACTION="-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@1.0", failed due
to this.

AllocateUnreservedBuffer() consumes memory reservation of the operator.
When transferring data to downstream operators, the used reservation
needs to be transferred together. This is a TODO item of this patch.
It's a problem in MT_DOP=0, i.e. HdfsScanNode, since
HdfsScanNode::ReturnReservationFromScannerThread() can't return the
reservation requested for the scanner thread. Reservation that
previously used by IO buffers might be used in the output RowBatch so
can't be returned when the scanner thread is closed. No such problem in
MT_DOP>0 where HdfsScanNode::ReturnReservationFromScannerThread() is not
used. The current patch still uses the old MemPool mode when MT_DOP=0.

TODO: test ORC scanner

Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
---
M be/src/exec/exec-node.h
M be/src/exec/hdfs-columnar-scanner.cc
M be/src/exec/hdfs-columnar-scanner.h
M be/src/exec/hdfs-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.h
M be/src/exec/parquet/parquet-column-chunk-reader.cc
M be/src/exec/scratch-tuple-batch.h
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/io/request-ranges.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
18 files changed, 259 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/22123/5
-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22123 )

Change subject: WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/19649/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jun 2025 10:00:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22123 )

Change subject: WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner
......................................................................


Patch Set 1:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/17589/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Nov 2024 12:41:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22123 )

Change subject: WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner
......................................................................


Patch Set 4:

(4 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/22123/4/be/src/exec/hdfs-columnar-scanner.cc
File be/src/exec/hdfs-columnar-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/22123/4/be/src/exec/hdfs-columnar-scanner.cc@322
PS4, Line 322:   VLOG_QUERY << "ideal_reservation=" << ideal_io_reservation << ", total_reservation=" << context_->total_reservation();
line too long (120 > 90)


http://gerrit.cloudera.org:8080/#/c/22123/4/be/src/exec/hdfs-columnar-scanner.cc@330
PS4, Line 330:   int64_t reservation_to_distribute = min(ideal_io_reservation, context_->total_reservation());
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/22123/4/be/src/exec/hdfs-columnar-scanner.cc@332
PS4, Line 332:       context_->total_reservation() - extra_required_reservation - bp_client_->GetUsedReservation());
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/22123/4/be/src/exec/hdfs-columnar-scanner.cc@333
PS4, Line 333:   VLOG_QUERY << "reservation_to_distribute=" << reservation_to_distribute << ", total_reservation=" << context_->total_reservation();
line too long (133 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jun 2025 03:04:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22123 )

Change subject: WIP: IMPALA-13486: Support MemBool backed by BufferPool in parquet scanner
......................................................................


Patch Set 1:

(1 comment)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python2 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with virtualenv installed.

http://gerrit.cloudera.org:8080/#/c/22123/1/be/src/exec/hdfs-scan-node.cc
File be/src/exec/hdfs-scan-node.cc:

http://gerrit.cloudera.org:8080/#/c/22123/1/be/src/exec/hdfs-scan-node.cc@541
PS1, Line 541:              << " scanner->GetUsedReservationInRowBatch()=" << scanner->GetUsedReservationInRowBatch();
line too long (103 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Nov 2024 12:25:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/22123

to look at the new patch set (#3).

Change subject: WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner
......................................................................

WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

MemPool is used as an Arena that pre-allocates memory chunks to save
frequent calls on malloc() and free() from the thread. It's helpful
especially for small objects that have a similar lifetime and fit into
the pre-allocated chunks. However, when allocating a large memory space
that no free chunks can fit, it still invokes malloc() to allocate the
space.

TCMalloc is used in Impala as the memory allocator. It has a ThreadCache
for each thread to serve small allocations. However, for allocations
larger than 256KB, they are served by the Central Cache of TCMalloc
which has lock contention on its CentralFreeList.

Currently, when scanner threads allocate large memory spaces from
MemPool, they suffer from this TCMalloc contention. Performance degrades
as concurrency increases.

BufferPool is another memory management layer in Impala to handle memory
reservation and spill-to-disk for all queries. It also enables reuse of
buffers (memory spaces) between queries, to avoid frequent allocations.
Allocating large buffers from BufferPool won't hit the above TCMalloc
contention issue if the allocations can be served by previously freed
buffers.

This patch extends MemPool to be able to allocate memory chunks using
BufferPool. A new constructor passing a BufferPool client is added to
enable this mode. MemPool now manages a list of memory chunks allocated
from malloc() and a list of buffers allocated from BufferPool. MemPools
in different modes can acquire data from each other, by updating these
two lists.

ScratchTupleBatch is used in Parquet/ORC scanners to materialize tuples
before evaluating filters. It currently has two MemPools, tuple_mem_pool
and aux_mem_pool, for the fix-sized and var-len parts respectively.
ParquetColumnChunkReader has a data_page_pool_ which allocates memory
for decompressed/copied Parquet data pages. These three kinds of
MemPools are where a scanner could allocate large memory space. They are
now backed by BufferPool to avoid the above TCMalloc contention issue.

----------
Limitation

When allocating memory from BufferPool, MemPool uses
AllocateUnreservedBuffer() which might increase memory reservation in
runtime to fit the space that used to be allocated from malloc(). This
makes the query easier to hit OOM when it couldn't increase the
reservation. Some tests that expect the query can run with the minimal
reservation, i.e.
DEBUG_ACTION="-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@1.0", failed due
to this. TODO: planner should increase the minimal reservation for this.

AllocateUnreservedBuffer() consumes memory reservation of the operator.
When transferring data to downstream operators, the used reservation
needs to be transferred together. This is a TODO item of this patch.
It's a problem in MT_DOP=0, i.e. HdfsScanNode, since
HdfsScanNode::ReturnReservationFromScannerThread() can't return the
reservation requested for the scanner thread. Reservation that
previously used by IO buffers might be used in the output RowBatch so
can't be returned when the scanner thread is closed. No such problem in
MT_DOP>0 where HdfsScanNode::ReturnReservationFromScannerThread() is not
used. The current patch still uses the old MemPool mode when MT_DOP=0.

TODO: test ORC scanner

Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
---
M be/src/exec/exec-node.h
M be/src/exec/hdfs-columnar-scanner.cc
M be/src/exec/hdfs-columnar-scanner.h
M be/src/exec/hdfs-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.h
M be/src/exec/parquet/parquet-column-chunk-reader.cc
M be/src/exec/scratch-tuple-batch.h
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
16 files changed, 211 insertions(+), 54 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/22123/3
-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Zoltan Borok-Nagy, Csaba Ringhofer, Michael Smith, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/22123

to look at the new patch set (#4).

Change subject: WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner
......................................................................

WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

MemPool is used as an Arena that pre-allocates memory chunks to save
frequent calls on malloc() and free() from the thread. It's helpful
especially for small objects that have a similar lifetime and fit into
the pre-allocated chunks. However, when allocating a large memory space
that no free chunks can fit, it still invokes malloc() to allocate the
space.

TCMalloc is used in Impala as the memory allocator. It has a ThreadCache
for each thread to serve small allocations. However, for allocations
larger than 256KB, they are served by the Central Cache of TCMalloc
which has lock contention on its CentralFreeList.

Currently, when scanner threads allocate large memory spaces from
MemPool, they suffer from this TCMalloc contention. Performance degrades
as concurrency increases.

BufferPool is another memory management layer in Impala to handle memory
reservation and spill-to-disk for all queries. It also enables reuse of
buffers (memory spaces) between queries, to avoid frequent allocations.
Allocating large buffers from BufferPool won't hit the above TCMalloc
contention issue if the allocations can be served by previously freed
buffers.

This patch extends MemPool to be able to allocate memory chunks using
BufferPool. A new constructor passing a BufferPool client is added to
enable this mode. MemPool now manages a list of memory chunks allocated
from malloc() and a list of buffers allocated from BufferPool. MemPools
in different modes can acquire data from each other, by updating these
two lists.

ScratchTupleBatch is used in Parquet/ORC scanners to materialize tuples
before evaluating filters. It currently has two MemPools, tuple_mem_pool
and aux_mem_pool, for the fix-sized and var-len parts respectively.
ParquetColumnChunkReader has a data_page_pool_ which allocates memory
for decompressed/copied Parquet data pages. These three kinds of
MemPools are where a scanner could allocate large memory space. They are
now backed by BufferPool to avoid the above TCMalloc contention issue.

Min reservation of the HdfsScanNode is increased for these allocation.

----------
Limitation

When allocating memory from BufferPool, MemPool uses
AllocateUnreservedBuffer() which might increase memory reservation in
runtime to fit the space that used to be allocated from malloc(). This
makes the query easier to hit OOM when it couldn't increase the
reservation. Some tests that expect the query can run with the minimal
reservation, i.e.
DEBUG_ACTION="-1:OPEN:SET_DENY_RESERVATION_PROBABILITY@1.0", failed due
to this.

AllocateUnreservedBuffer() consumes memory reservation of the operator.
When transferring data to downstream operators, the used reservation
needs to be transferred together. This is a TODO item of this patch.
It's a problem in MT_DOP=0, i.e. HdfsScanNode, since
HdfsScanNode::ReturnReservationFromScannerThread() can't return the
reservation requested for the scanner thread. Reservation that
previously used by IO buffers might be used in the output RowBatch so
can't be returned when the scanner thread is closed. No such problem in
MT_DOP>0 where HdfsScanNode::ReturnReservationFromScannerThread() is not
used. The current patch still uses the old MemPool mode when MT_DOP=0.

TODO: test ORC scanner

Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
---
M be/src/exec/exec-node.h
M be/src/exec/hdfs-columnar-scanner.cc
M be/src/exec/hdfs-columnar-scanner.h
M be/src/exec/hdfs-scanner.h
M be/src/exec/parquet/hdfs-parquet-scanner.cc
M be/src/exec/parquet/hdfs-parquet-scanner.h
M be/src/exec/parquet/parquet-column-chunk-reader.cc
M be/src/exec/scratch-tuple-batch.h
M be/src/runtime/bufferpool/buffer-pool.h
M be/src/runtime/bufferpool/reservation-tracker.cc
M be/src/runtime/io/request-ranges.h
M be/src/runtime/mem-pool.cc
M be/src/runtime/mem-pool.h
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/runtime/row-batch.cc
M be/src/runtime/row-batch.h
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
18 files changed, 253 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/22123/4
-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22123 )

Change subject: WIP: IMPALA-13486: Support MemPool backed by BufferPool in parquet scanner
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/19648/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/22123
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cf0eac43fa98cb4cff66e5061f5bb561487d6ab
Gerrit-Change-Number: 22123
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jun 2025 03:29:04 +0000
Gerrit-HasComments: No