You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org> on 2024/12/10 13:09:17 UTC

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Noemi Pap-Takacs has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22189


Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................

IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Iceberg supports multiple writers with optimistic concurrency.
Each writer can write new files which are then added to the table
after a validation check to ensure that the commit does not conflict
with other modifications made during the execution.

When there was a conflicting change which cannot be resolved, it
means that the newly written files cannot be committed to the table,
so they used to become orphan files in the file system.

This change introduces automatic cleanup of uncommitted files
after an unsuccessful DML operation.

Testing:
- E2E test: injected ValidationException with debug option

Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_iceberg.py
4 files changed, 82 insertions(+), 3 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 1:

(5 comments)

Thanks for this patch, it looks good. I only have a few questions.

http://gerrit.cloudera.org:8080/#/c/22189/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22189/1//COMMIT_MSG@14
PS1, Line 14: cannot
Nit: "could not".


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7564
PS1, Line 7564: ImpalaRuntimeException
Do we need to catch ImpalaRuntimeException here? Can a validation error also cause an ImpalaRuntimeException instead of a ValidationException? Or is ValidationException only used in the debug action?


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@608
PS1, Line 608:     List<String> pathStrings = new ArrayList<>();
Is there a reason for first collecting the paths and then deleting them? Couldn't we delete them "on-the-fly" in the FOR loops?


http://gerrit.cloudera.org:8080/#/c/22189/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/22189/1/tests/query_test/test_iceberg.py@1999
PS1, Line 1999:       table = "iceberg_cleanup_failure"
Instead of 'table' and 'tbl_name', I think 'tbl_name' and 'fq_table_name' (or something similar) would be more correct as these two variables both hold table names.


http://gerrit.cloudera.org:8080/#/c/22189/1/tests/query_test/test_iceberg.py@2007
PS1, Line 2007:       self.client.execute("""CREATE TABLE {0} (i int)
Is there a reason why this is not "execute_query_expect_success()"?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Tue, 10 Dec 2024 16:48:39 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 3: Code-Review+1

(2 comments)

LGTM!

http://gerrit.cloudera.org:8080/#/c/22189/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7561
PS3, Line 7561:       DebugUtils.executeDebugAction(
              :           update.getDebug_action(), DebugUtils.ICEBERG_CONFLICT);
nit: this culd be moved before execute()

* so it will be consistent with DebugUtils.ICEBERG_COMMIT
* we won't create manifest files unnecessarily


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@607
PS1, Line 607: 
> In case of exceptions, the Catalog timeline is not visible in the query pro
I see, thanks for investigating this.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 3
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Dec 2024 14:25:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22189/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7588
PS5, Line 7588:       throw new ImpalaRuntimeException(e.getMessage(), e);
I was wondering whether it would be useful to log the names of the files that were cleaned up - this could be good for example in case the table is corrupted because of missing files: we could rule out that this cleanup was the one that (incorrectly) deleted the file, or we could confirm if there is a bug with the cleanup.

The argument against it is that it could potentially be hundreds of files. Do you know if it is possible to recover this info from filesystem logs as well? Do they show that it was Impala that deleted these files?


http://gerrit.cloudera.org:8080/#/c/22189/5/tests/stress/test_update_stress.py
File tests/stress/test_update_stress.py:

http://gerrit.cloudera.org:8080/#/c/22189/5/tests/stress/test_update_stress.py@315
PS5, Line 315:     assert len(data_files_in_tbl.data) == len(data_files_on_fs_rows) - 1
Would it be difficult to check that the files are actually the same, not only the number of files?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 5
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jan 2025 12:27:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 1:

(4 comments)

Thanks for working on this!

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7560
PS1, Line 7560: if (update.isSetDebug_action())
nit: I don't think we need this if stmt as NULL and empty values are handled by DebugUtils.executeDebugAction().


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7585
PS1, Line 7585:     iceTxn.commitTransaction();
> I'm a bit confused now how this works in general. Do we make the actual com
L7565 is IcebergCatalogOpExecutor.cleanupUncommittedFiles(). Did you mean L7562? In that case, AFAICT a new snapshot file (or even multiple snapshot files) is written, then we commit all the changes here, i.e. this is the point that sets the new table metadata JSON. Up until to this point only new files are being created, but they are all uncommited i.e. they don't have effect on the table's state.


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7587
PS1, Line 7587:   }
> I'm wondering if it makes sense to repro the "uncleaned files" scenario by 
I think we already have such stress tests in tests/stress/. In the end we could compare the contents of the data/ directory and the `files` metadata table.


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@607
PS1, Line 607:   protected static void cleanupUncommittedFiles(TIcebergOperationParam icebergOp) {
We could add catalogTimeline events to this method, see e.g. CatalogOpExecutor.insertIntoIcebergTable()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@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, 11 Dec 2024 10:23:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@621
PS1, Line 621:       deleteIfExists(path);
> In theory deleteIfExists can throw an IOException. So if it does for some r
In fact deleteIfxists() swallows IOException. We most probably are fine here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@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, 11 Dec 2024 09:05:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/22189 )

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................

IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Iceberg supports multiple writers with optimistic concurrency.
Each writer can write new files which are then added to the table
after a validation check to ensure that the commit does not conflict
with other modifications made during the execution.

When there was a conflicting change which could not be resolved, it
means that the newly written files cannot be committed to the table,
so they used to become orphan files on the file system. Orphan files
can accumulate over time, taking up a lot of storage space. They do
not belong to the table because they are not referenced by any snapshot
and therefore they can't be removed by expiring snapshots.

This change introduces automatic cleanup of uncommitted files
after an unsuccessful DML operation to prevent creating orphan files.
No cleanup is done if Iceberg throws CommitStateUnknownException
because the update success or failure is unknown in this case.

Testing:
- E2E test: Injected ValidationException with debug option.
- stress test: Added a method to check that no orphan files were
  created after failed conflicting commits.

Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_iceberg.py
M tests/stress/test_update_stress.py
5 files changed, 157 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/22189/8
-- 
To view, visit http://gerrit.cloudera.org:8080/22189
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 8
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 5
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jan 2025 11:13:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/22189 )

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................

IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Iceberg supports multiple writers with optimistic concurrency.
Each writer can write new files which are then added to the table
after a validation check to ensure that the commit does not conflict
with other modifications made during the execution.

When there was a conflicting change which could not be resolved, it
means that the newly written files cannot be committed to the table,
so they used to become orphan files on the file system. Orphan files
can accumulate over time, taking up a lot of storage space. They do
not belong to the table because they are not referenced by any snapshot
and therefore they can't be removed by expiring snapshots.

This change introduces automatic cleanup of uncommitted files
after an unsuccessful DML operation to prevent creating orphan files.
No cleanup is done if Iceberg throws CommitStateUnknownException
because the update success or failure is unknown in this case.

Testing:
- E2E test: Injected ValidationException with debug option.
- stress test: Added a method to check that no orphan files were
  created after failed conflicting commits.

Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_iceberg.py
M tests/stress/test_update_stress.py
5 files changed, 157 insertions(+), 63 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/22189/6
-- 
To view, visit http://gerrit.cloudera.org:8080/22189
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 6
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 8
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Jan 2025 18:52:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 1:

Build Failed 

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Tue, 10 Dec 2024 23:36:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7585
PS1, Line 7585:     iceTxn.commitTransaction();
> L7565 is IcebergCatalogOpExecutor.cleanupUncommittedFiles(). Did you mean L
Hmm, I think I was meant to write L7559. With this implementation we clean up files if there is an exception there. M point is that do we also make this clean up here if committing the transaction fails.


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@605
PS1, Line 605:    *                  data and delete files which have to be cleaned up.
> I don't know if it is consistent with our coding guide, but I find it logic
In general it's 4 spaces for a line break.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@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, 11 Dec 2024 20:13:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 8
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Jan 2025 13:17:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/22189 )

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................

IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Iceberg supports multiple writers with optimistic concurrency.
Each writer can write new files which are then added to the table
after a validation check to ensure that the commit does not conflict
with other modifications made during the execution.

When there was a conflicting change which could not be resolved, it
means that the newly written files cannot be committed to the table,
so they used to become orphan files on the file system. Orphan files
can accumulate over time, taking up a lot of storage space. They do
not belong to the table because they are not referenced by any snapshot
and therefore they can't be removed by expiring snapshots.

This change introduces automatic cleanup of uncommitted files
after an unsuccessful DML operation to prevent creating orphan files.
No cleanup is done if Iceberg throws CommitStateUnknownException
because the update success or failure is unknown in this case.

Testing:
- E2E test: Injected ValidationException with debug option.
- stress test: Added a method to check that no orphan files were
  created after failed conflicting commits.

Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_iceberg.py
M tests/stress/test_update_stress.py
5 files changed, 151 insertions(+), 63 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 3
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 6
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jan 2025 11:39:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/22189 )

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................

IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Iceberg supports multiple writers with optimistic concurrency.
Each writer can write new files which are then added to the table
after a validation check to ensure that the commit does not conflict
with other modifications made during the execution.

When there was a conflicting change which could not be resolved, it
means that the newly written files cannot be committed to the table,
so they used to become orphan files in the file system.

This change introduces automatic cleanup of uncommitted files
after an unsuccessful DML operation.

Testing:
- E2E test: injected ValidationException with debug option
- TODO: stress test

Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_iceberg.py
4 files changed, 124 insertions(+), 60 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 2
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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/22189 )

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................

IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Iceberg supports multiple writers with optimistic concurrency.
Each writer can write new files which are then added to the table
after a validation check to ensure that the commit does not conflict
with other modifications made during the execution.

When there was a conflicting change which could not be resolved, it
means that the newly written files cannot be committed to the table,
so they used to become orphan files on the file system. Orphan files
can accumulate over time, taking up a lot of storage space. They do
not belong to the table because they are not referenced by any snapshot
and therefore they can't be removed by expiring snapshots.

This change introduces automatic cleanup of uncommitted files
after an unsuccessful DML operation to prevent creating orphan files.
No cleanup is done if Iceberg throws CommitStateUnknownException
because the update success or failure is unknown in this case.

Testing:
- E2E test: Injected ValidationException with debug option.
- stress test: Added a method to check that no orphan files were
  created after failed conflicting commits.

Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Reviewed-on: http://gerrit.cloudera.org:8080/22189
Reviewed-by: Daniel Becker <da...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_iceberg.py
M tests/stress/test_update_stress.py
5 files changed, 157 insertions(+), 63 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 9
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Tue, 10 Dec 2024 13:35:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22189/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7575
PS2, Line 7575:       DebugUtils.executeDebugAction(update.getDebug_action(), DebugUtils.ICEBERG_COMMIT);
There are two DebugUtils.executeDebugAction() in this function, and I think in cases where the second one would be triggered, the first will also be triggered, so the second will never actually run.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 2
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Dec 2024 14:21:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 7
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jan 2025 22:20:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 8
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Jan 2025 13:18:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@605
PS1, Line 605:    *                  data and delete files which have to be cleaned up.
> nit: this seems an odd indentation
I don't know if it is consistent with our coding guide, but I find it logical: 'icebergOp' is the name of the parameter, the explanation is indented to come after that.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@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, 11 Dec 2024 11:42:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 2
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Dec 2024 16:03:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 6: Code-Review+2

LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 6
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jan 2025 16:53:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 8:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 8
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Jan 2025 13:30:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 4
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jan 2025 11:06:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/22189 )

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................

IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Iceberg supports multiple writers with optimistic concurrency.
Each writer can write new files which are then added to the table
after a validation check to ensure that the commit does not conflict
with other modifications made during the execution.

When there was a conflicting change which could not be resolved, it
means that the newly written files cannot be committed to the table,
so they used to become orphan files on the file system. Orphan files
can accumulate over time, taking up a lot of storage space. They do
not belong to the table because they are not referenced by any snapshot
and therefore they can't be removed by expiring snapshots.

This change introduces automatic cleanup of uncommitted files
after an unsuccessful DML operation to prevent creating orphan files.
No cleanup is done if Iceberg throws CommitStateUnknownException
because the update success or failure is unknown in this case.

Testing:
- E2E test: Injected ValidationException with debug option.
- stress test: Added a method to check that no orphan files were
  created after failed conflicting commits.

Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_iceberg.py
M tests/stress/test_update_stress.py
5 files changed, 150 insertions(+), 63 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 4
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 2:

(2 comments)

The code looks good to me, looking forward for the stress test enhancements.

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@607
PS1, Line 607: }
> Is it ok to add it in CatalogOpExecutor after calling this function?
I think it's fine until we start using this at other places where we have EventSequences.


http://gerrit.cloudera.org:8080/#/c/22189/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/2/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@599
PS2, Line 599:       for (ByteBuffer buf : icebergOp.getIceberg_delete_files_fb()) {
             :         String pathString = FbIcebergDataFile.getRootAsFbIcebergDataFile(buf).path();
             :         if (pathString != null) {
             :           deleteIfExists(new org.apache.hadoop.fs.Path((pathString)));
             :         }
             :       }
nit: this duplicates L591-L597. Could be moved to a method that takes an Iterable<ByteBuffer> as parameter.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 2
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Dec 2024 17:57:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 3
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Dec 2024 14:02:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 1:

(10 comments)

Thanks for the changes, Noemi! This is an important feature to have IMO

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7561
PS1, Line 7561:         String debugAction = update.getDebug_action();
nit: would it fit into a single line if you merged this one with the one below?


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7564
PS1, Line 7564: ImpalaRuntimeException
> Do we need to catch ImpalaRuntimeException here? Can a validation error als
I think SnapshotProducer.commit() could also throw a CommitFailedException and the validate*() functions, also getting IOexception during the validation check could result in throwing and UncheckedIOException (that extends RuntimeException). I have the impression that we can get a lot of different exceptions here and since Iceberg uses RuntimeExceptions it's not that easy to track all of them down.
I think the safest approach is to catch all Exceptions here. At the end of the day it doesn't matter what failed the commit/validations, we have to clean up anyway.


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7585
PS1, Line 7585:     iceTxn.commitTransaction();
I'm a bit confused now how this works in general. Do we make the actual commit of the new snapshot to the table here? Do do we commit the new snapshot in L7565?
If this one can also throw exceptions then we don't do the cleanup here.


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7587
PS1, Line 7587:   }
I'm wondering if it makes sense to repro the "uncleaned files" scenario by not the debug actions but with actually setting up a test where multiple updates/deletes race against each other.


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@605
PS1, Line 605:    *                  data and delete files which have to be cleaned up.
nit: this seems an odd indentation


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@621
PS1, Line 621:       deleteIfExists(path);
In theory deleteIfExists can throw an IOException. So if it does for some reason for a file at the middle of the list, we won't delete the rest. Wondering how likely this is, and if we should handle such cases.
I recall the validation*() functions in Iceberg could also fail with IOExceptions, so the same root cause might result in an IOException here too.


http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/util/DebugUtils.java
File fe/src/main/java/org/apache/impala/util/DebugUtils.java:

http://gerrit.cloudera.org:8080/#/c/22189/1/fe/src/main/java/org/apache/impala/util/DebugUtils.java@196
PS1, Line 196:               exceptionToThrow = new ValidationException(param);
Do I get it right that this is introduced to simulate when a new snapshot can't be committed due to conflicts with other parallel commits?
I think in that case Iceberg throws a CommitFailedException:
https://github.com/apache/iceberg/blob/ff813445916bfd6ec1cc30a02b02f8bade7a26f6/core/src/main/java/org/apache/iceberg/SnapshotProducer.java#L403


http://gerrit.cloudera.org:8080/#/c/22189/1/tests/query_test/test_iceberg.py
File tests/query_test/test_iceberg.py:

http://gerrit.cloudera.org:8080/#/c/22189/1/tests/query_test/test_iceberg.py@2018
PS1, Line 2018:       result = str(err)
No need to introduce a 'result' variable here


http://gerrit.cloudera.org:8080/#/c/22189/1/tests/query_test/test_iceberg.py@2020
PS1, Line 2020:                                 "ValidationException: simulated validation check failure")
nit: indentation seems to be off compared to other line breaks


http://gerrit.cloudera.org:8080/#/c/22189/1/tests/query_test/test_iceberg.py@2032
PS1, Line 2032:       assert "Found 1 items" in files_result
I get that metadata file removal is performed by the Iceberg lib, but I think we can add a check for that too. With that we could catch if the library is broken after a version bump.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <np...@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, 11 Dec 2024 08:46:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 4:

(2 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/22189/4/tests/stress/test_update_stress.py
File tests/stress/test_update_stress.py:

http://gerrit.cloudera.org:8080/#/c/22189/4/tests/stress/test_update_stress.py@243
PS4, Line 243:  
flake8: E222 multiple spaces after operator


http://gerrit.cloudera.org:8080/#/c/22189/4/tests/stress/test_update_stress.py@315
PS4, Line 315: 
flake8: W292 no newline at end of file



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 4
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jan 2025 10:41:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/22189 )

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................

IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

Iceberg supports multiple writers with optimistic concurrency.
Each writer can write new files which are then added to the table
after a validation check to ensure that the commit does not conflict
with other modifications made during the execution.

When there was a conflicting change which could not be resolved, it
means that the newly written files cannot be committed to the table,
so they used to become orphan files on the file system. Orphan files
can accumulate over time, taking up a lot of storage space. They do
not belong to the table because they are not referenced by any snapshot
and therefore they can't be removed by expiring snapshots.

This change introduces automatic cleanup of uncommitted files
after an unsuccessful DML operation to prevent creating orphan files.
No cleanup is done if Iceberg throws CommitStateUnknownException
because the update success or failure is unknown in this case.

Testing:
- E2E test: Injected ValidationException with debug option.
- stress test: Added a method to check that no orphan files were
  created after failed conflicting commits.

Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/DebugUtils.java
M tests/query_test/test_iceberg.py
M tests/stress/test_update_stress.py
5 files changed, 150 insertions(+), 63 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 5
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 6: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 6
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Jan 2025 11:23:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 7
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jan 2025 16:53:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure

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

Change subject: IMPALA-13501: Clean up uncommitted Iceberg files after validation check failure
......................................................................


Patch Set 7: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibe59546ebf3c639b75b53dfa1daba37cef50eb21
Gerrit-Change-Number: 22189
Gerrit-PatchSet: 7
Gerrit-Owner: Noemi Pap-Takacs <np...@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: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Jan 2025 16:53:26 +0000
Gerrit-HasComments: No