You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2022/09/23 14:48:32 UTC

[Impala-ASF-CR] IMPALA-11508: Deflake test expire snapshots

Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19036


Change subject: IMPALA-11508: Deflake test_expire_snapshots
......................................................................

IMPALA-11508: Deflake test_expire_snapshots

Before this patch test_expire_snapshots failed frequently. The patch is
only a few lines of code, but there are some subtleties here:

IcebergCatalogOpExecutor.alterTableExecute() didn't use Iceberg
transactions to carry out the operation. This means that
expireSnapshots() resulted in an ALTER TABLE operation on its own
which we also got during event processing.

Because this ALTER TABLE event didn't had the catalog version set we
didn't recognized it as a self-event. This caused unnecessary table
reloads during the tests which manifested in
InconsistentMetadataFetchException: "... table ... changed version
between accesses" errors.

With this patch IcebergCatalogOpExecutor.alterTableExecute() takes
an Iceberg transaction object and invokes expireSnapshots() in the
context of this Iceberg transaction. This Iceberg transaction also
sets table properties "impala.events.catalogServiceId" and
"impala.events.catalogVersion". And because everything happens in
a single Iceberg transaction we only create a single ALTER TABLE
which we can recognize during event processing (based on the
table properties), avoiding unnecessary table reloads.

Testing:
 * executed test_expire_snapshots in a loop

Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
2 files changed, 3 insertions(+), 3 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Gerrit-Change-Number: 19036
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-11508: Deflake test expire snapshots

Posted by "Gergely Fürnstáhl (Code Review)" <ge...@cloudera.org>.
Gergely Fürnstáhl has posted comments on this change. ( http://gerrit.cloudera.org:8080/19036 )

Change subject: IMPALA-11508: Deflake test_expire_snapshots
......................................................................


Patch Set 1: Code-Review+1

Looks good to me!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Gerrit-Change-Number: 19036
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Mon, 26 Sep 2022 09:09:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11508: Deflake test expire snapshots

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

Change subject: IMPALA-11508: Deflake test_expire_snapshots
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Gerrit-Change-Number: 19036
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Mon, 26 Sep 2022 10:31:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11508: Deflake test expire snapshots

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

Change subject: IMPALA-11508: Deflake test_expire_snapshots
......................................................................


Patch Set 1: Code-Review+2

Nice fix, LGTM!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Gerrit-Change-Number: 19036
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Mon, 26 Sep 2022 09:47:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11508: Deflake test expire snapshots

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

Change subject: IMPALA-11508: Deflake test_expire_snapshots
......................................................................

IMPALA-11508: Deflake test_expire_snapshots

Before this patch test_expire_snapshots failed frequently. The patch is
only a few lines of code, but there are some subtleties here:

IcebergCatalogOpExecutor.alterTableExecute() didn't use Iceberg
transactions to carry out the operation. This means that
expireSnapshots() resulted in an ALTER TABLE operation on its own
which we also got during event processing.

Because this ALTER TABLE event didn't had the catalog version set we
didn't recognized it as a self-event. This caused unnecessary table
reloads during the tests which manifested in
InconsistentMetadataFetchException: "... table ... changed version
between accesses" errors.

With this patch IcebergCatalogOpExecutor.alterTableExecute() takes
an Iceberg transaction object and invokes expireSnapshots() in the
context of this Iceberg transaction. This Iceberg transaction also
sets table properties "impala.events.catalogServiceId" and
"impala.events.catalogVersion". And because everything happens in
a single Iceberg transaction we only create a single ALTER TABLE
which we can recognize during event processing (based on the
table properties), avoiding unnecessary table reloads.

Testing:
 * executed test_expire_snapshots in a loop

Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Reviewed-on: http://gerrit.cloudera.org:8080/19036
Reviewed-by: Impala Public Jenkins <im...@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
2 files changed, 3 insertions(+), 3 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Gerrit-Change-Number: 19036
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>

[Impala-ASF-CR] IMPALA-11508: Deflake test expire snapshots

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

Change subject: IMPALA-11508: Deflake test_expire_snapshots
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Gerrit-Change-Number: 19036
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Mon, 26 Sep 2022 10:31:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11508: Deflake test expire snapshots

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

Change subject: IMPALA-11508: Deflake test_expire_snapshots
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Gerrit-Change-Number: 19036
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Fri, 23 Sep 2022 15:09:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11508: Deflake test expire snapshots

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

Change subject: IMPALA-11508: Deflake test_expire_snapshots
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15
Gerrit-Change-Number: 19036
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Comment-Date: Mon, 26 Sep 2022 15:40:53 +0000
Gerrit-HasComments: No