Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18191 )

Change subject: IMPALA-9433: Improved caching of HdfsFileHandles
......................................................................


Patch Set 24: Code-Review+1

(1 comment)

This is looking good to me.

Did we do a targeted performance test? One example would be to make a Parquet 
table with a large number of files with lots of files. In past work, I took the 
Parquet file from functional_parquet.widetable_1000_cols and created a table 
with 1000 copies of that file (copy the table schema by using "create table 
like"). A select * on that table would require a large number of file handle 
cache accesses.

http://gerrit.cloudera.org:8080/#/c/18191/21/be/src/util/lru-multi-cache.h
File be/src/util/lru-multi-cache.h:

http://gerrit.cloudera.org:8080/#/c/18191/21/be/src/util/lru-multi-cache.h@170
PS21, Line 170:   typedef std::list<ValueType_internal> Container;
              :
> I've considered moving this to the accessor, but decided to keep it here to
I haven't tried this, but here is something that might work:
1. Add a boolean variable indicating whether the Accessor is valid
2. Add an Accessor constructor that takes no arguments. It initializes valid to 
false and doesn't do anything to the iterator. The iterator is in an undefined 
state, so we can't use it. For the valid=false case, we shouldn't need to use 
it.
3. Modify the other constructor to always set valid=true and set the iterator.
4. Modify code to treat valid=false the same as the null case today.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b5c5e9e2b5db2847ab88c41f667c9ca1b03d51a
Gerrit-Change-Number: 18191
Gerrit-PatchSet: 24
Gerrit-Owner: Gergely Fürnstáhl <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Fri, 25 Feb 2022 03:10:22 +0000
Gerrit-HasComments: Yes

Reply via email to