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

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Gabor Kaszab has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21847


Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................

IMPALA-11265: Part1: Clear GroupContentFiles once used

GroupContentFiles stores the file descriptors in Iceberg's format and is
used for creating file descriptors in Impala's format. Once this
creation is done, we no longer have to keep these Iceberg ContentFiles.
Cleaning these could significantly reduce the mempry footprint of an
Iceberg table.

Measurements:
I have a test table that has 110k files. The measurements showed that
cleaning the GroupedContentFiles could reduce the memory size of this
particular table from 140MB to 80MB.

Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
---
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java
2 files changed, 25 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21847 )

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 2: Code-Review+2

LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2024 16:07:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/21847 )

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@427
PS1, Line 427:         // We no longer need to keep Iceberg's content files in memory.
             :         hdfsTable_.setIceberg
> Actually, can't we get rid of the 'icebergFiles_' member in HdfsTable if it
Well unsetting 'icebergFile_' seems in fact better than how I did it. I don't know why I haven't thought about that :)

Daniel: Technically we could get rid of that member by passing it to the load() and all underlying functions, but since load() is an override, we might have to add this to many places that seems an overkill.
The ultimate solution would be to eventually get rid of the 'hdfsTable_' from IceebergTable and let IcebergTable load its partitions and file descriptors and don't rely on HdfsTable for that.


http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java:

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java@92
PS1, Line 92: 
> Isn't it that it would delete the contents of the collections but would not
I checked, if you call clear() it sets all the items in the ArrayList to null, but would keep their place in the ArrayList. So after calling clear(), with the 110k files example I had these containers still took ~600k memory because they still held 110k references (that were in fact nulls).
We could set them to nulls, yes, but with the new approach in PS2 this doesn't matter anymore.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2024 09:41:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

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

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/17015/ : 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/21847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2024 15:21:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21847 )

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@427
PS1, Line 427:         // We no longer need to keep Iceberg's content files in memory.
             :         icebergFiles.clear();
> Unsetting 'icebergFiles_' seems better to me because having an empty 'icebe
Actually, can't we get rid of the 'icebergFiles_' member in HdfsTable if it is only used for a short timeframe? We could pass it to the function(s) that use it as an argument.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2024 15:27:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21847 )

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@427
PS1, Line 427:         // We no longer need to keep Iceberg's content files in memory.
             :         hdfsTable_.setIceberg
> Well unsetting 'icebergFile_' seems in fact better than how I did it. I don
Ok, changing an overloaded function could be too much effort.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2024 10:54:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

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

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2024 07:30:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21847 )

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

I proposed an alternative solution, but I'm also OK with the current one. Thanks for catching and fixing this, Gabor!

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@427
PS1, Line 427:         // We no longer need to keep Iceberg's content files in memory.
             :         icebergFiles.clear();
Alternatively we could unset icebergFiles_ in hdfsTable_, so GC can collect it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2024 14:59:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Daniel Becker, Zoltan Borok-Nagy, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................

IMPALA-11265: Part1: Clear GroupContentFiles once used

GroupContentFiles stores the file descriptors in Iceberg's format and is
used for creating file descriptors in Impala's format. Once this
creation is done, we no longer have to keep these Iceberg ContentFiles.
Dropping these could significantly reduce the memory footprint of an
Iceberg table.

Measurements:
I have a test table that has 110k files. The measurements showed that
cleaning the GroupedContentFiles could reduce the memory size of this
particular table from 140MB to 80MB.

Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
---
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java
2 files changed, 7 insertions(+), 0 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21847 )

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................

IMPALA-11265: Part1: Clear GroupContentFiles once used

GroupContentFiles stores the file descriptors in Iceberg's format and is
used for creating file descriptors in Impala's format. Once this
creation is done, we no longer have to keep these Iceberg ContentFiles.
Dropping these could significantly reduce the memory footprint of an
Iceberg table.

Measurements:
I have a test table that has 110k files. The measurements showed that
cleaning the GroupedContentFiles could reduce the memory size of this
particular table from 140MB to 80MB.

Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Reviewed-on: http://gerrit.cloudera.org:8080/21847
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java
2 files changed, 7 insertions(+), 0 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

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

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10976/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2024 07:30:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

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

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/17028/ : 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/21847
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2024 10:06:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

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

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2024 12:42:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11265: Part1: Clear GroupContentFiles once used

Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21847 )

Change subject: IMPALA-11265: Part1: Clear GroupContentFiles once used
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java:

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@427
PS1, Line 427:         // We no longer need to keep Iceberg's content files in memory.
             :         icebergFiles.clear();
> Alternatively we could unset icebergFiles_ in hdfsTable_, so GC can collect
Unsetting 'icebergFiles_' seems better to me because having an empty 'icebergFiles_' could be mistaken for a situation when there are actually no Iceberg files (e.g. empty table).


http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java:

http://gerrit.cloudera.org:8080/#/c/21847/1/fe/src/main/java/org/apache/impala/catalog/iceberg/GroupedContentFiles.java@92
PS1, Line 92: to null
Isn't it that it would delete the contents of the collections but would not set them to null.

Instead of creating new lists and sets, wouldn't it be better to set them to null, i.e. "dataFilesWithoutDeletes = null" etc.?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1efdd2a46c9675f7461535259e5892ed213a6b21
Gerrit-Change-Number: 21847
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Sep 2024 15:21:17 +0000
Gerrit-HasComments: Yes