You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2024/02/28 22:24:43 UTC

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

k.venureddy2103@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21087


Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

During AllocWriteIdEvent process, txnId to tableWriteIds mapping is
not added to catalog in the following cases:
1. When CREATE_TABLE event is followed by ALLOC_WRITE_ID_EVENT for
table in the same batch of MetastoreEventsProcessor.processEvents(),
process AllocWriteIdEvent do not add txnId to tableWriteIds mapping.
2. When table is not loaded.

This patch removes the usage of get table from catalog in the
event constructors. Also adds the txnId to tableWriteIds mapping in
catalog even when table is not loaded. And ensures the write ids
are not added to table if it is a non-partitioned table.

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 110 insertions(+), 39 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <k....@gmail.com>

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

k.venureddy2103@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 6:

(6 comments)

Reworked

http://gerrit.cloudera.org:8080/#/c/21087/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21087/4//COMMIT_MSG@11
PS4, Line 11: 1
> Isn't 1 a sub case of 2? After getting a CREATE_TABLE the table shouldn't b
In case-1, catalog table is not present because CREATE_TABLE event is not processed by the time of AllocWriteIdEvent construction. In case-2, catalog table is present but it is not loaded.

Have updated the description now.


http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3937
PS4, Line 3937:     if (tbl.getNumClusteringCols() == 0) {
> Can you add a comment with some explanation (or some pointer to an explanat
Done


http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3938
PS4, Line 3938:       // For non-partitioned tables, we just reload the whole table without keeping
> nit: missing "a"?
Done


http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2824
PS2, Line 2824:       // can be useful for other such events.
> msTbl_ is used inside super.isEventProcessingDisabled() below.
Added a TODO to have an init method to set fields that cannot be initialized in the event constructors and invoke it as the first step before processing event.


http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2824
PS4, Line 2824:  can b
> oops, I see that https://gerrit.cloudera.org/#/c/21087/2/fe/src/main/java/o
Yes. Have added a TODO for now. This gerrit is required for https://gerrit.cloudera.org/#/c/21065/. Shall take it up later  again as improvement.


http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3694
PS2, Line 3694:           "alter table " + TEST_DB_NAME + '.' + tblName + " compact 'minor' and wait");
> I might miss something. Do we already have tests in test_events_custom_conf
Added FE tests for both ReloadEvent and CommitCompactionEvent now



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 16:54:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 4:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Feb 2024 17:36:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 28 Feb 2024 22:41:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 7: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10345/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Mar 2024 08:10:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 10:

> Patch Set 10: Verified-1
> 
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10347/

This is tracked in IMPALA-9441 and is unrelated.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sun, 03 Mar 2024 02:27:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 2:

(3 comments)

Thanks for fixing this quickly! Just have some minor comments.

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2824
PS2, Line 2824:         msTbl_ = tbl.getMetaStoreTable();
Initializing this field here seems strange since this is a check method and is not expected to have side effects. I guess it's added here since this is the first method that uses this field. Maybe we need a Prepare or Finalize method to set fields before procesing the event. Could you add a TODO for this?


http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3659
PS2, Line 3659:   public void testAllocWriteIdEventAfterLoadTable(String tblName, boolean isPartitioned)
It seems we can consolidate this with testAllocWriteIdEvent() by adding a parameter of "boolean loadTable". The majority of the method codes are the same.


http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3694
PS2, Line 3694: 
Can we add tests for ReloadEvent and CommitCompactionEvent?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Feb 2024 11:09:56 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 16:56:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 10: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10347/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sun, 03 Mar 2024 02:06:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Mar 2024 21:26:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

During AllocWriteIdEvent process, txnId to tableWriteIds mapping is
not added to catalog in the following cases:
1. When CREATE_TABLE event is followed by ALLOC_WRITE_ID_EVENT for
the table in the same batch of MetastoreEventsProcessor.processEvents(),
process AllocWriteIdEvent cannot find catalog table since
CREATE_TABLE is not processed by the time of AllocWriteIdEvent
object construction.
2. When catalog table is present. But it is not loaded.

This patch fixes:
1. Removes the usage of get table from catalog in all the event
constructors. Currently, AllocWriteIdEvent, ReloadEvent,
CommitCompactionEvent get the catalog table in their constructors.
2. Adds the txnId to tableWriteIds mapping in catalog even when
table is not loaded. And ensures the write ids are not added to
table if it is a non-partitioned table.
3. Also fixed a bug in TableWriteId's hashCode() implementation that
is breaking hashcode contract. Two same TableWriteId of different
instances produce different hashcode though they are equal.
4. Fixed CatalogHmsSyncToLatestEventIdTest.cleanUp() issue.
flagInvalidateCache and flagSyncToLatestEventId are incorrectly set
in cleanUp.

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
M fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsSyncToLatestEventIdTest.java
5 files changed, 198 insertions(+), 42 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/21087/9
--
To view, visit http://gerrit.cloudera.org:8080/21087
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 10:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sun, 03 Mar 2024 02:28:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 7:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Mar 2024 03:28:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 8:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Mar 2024 12:04:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

k.venureddy2103@gmail.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 2:

(3 comments)

Reworked

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2824
PS2, Line 2824:         msTbl_ = tbl.getMetaStoreTable();
> Initializing this field here seems strange since this is a check method and
msTbl_ is used inside super.isEventProcessingDisabled() below.


http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3659
PS2, Line 3659:   public void testAllocWriteIdEventAfterLoadTable(String tblName, boolean isPartitioned)
> It seems we can consolidate this with testAllocWriteIdEvent() by adding a p
Done


http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3694
PS2, Line 3694: 
> Can we add tests for ReloadEvent and CommitCompactionEvent?
I think, tests covered in test_events_custom_configs.py are enough for these event.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Feb 2024 16:57:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

During AllocWriteIdEvent process, txnId to tableWriteIds mapping is
not added to catalog in the following cases:
1. When CREATE_TABLE event is followed by ALLOC_WRITE_ID_EVENT for
table in the same batch of MetastoreEventsProcessor.processEvents(),
process AllocWriteIdEvent do not add txnId to tableWriteIds mapping.
2. When table is not loaded.

This patch removes the usage of get table from catalog in the
event constructors. Also adds the txnId to tableWriteIds mapping in
catalog even when table is not loaded. And ensures the write ids
are not added to table if it is a non-partitioned table. Also fixed
a bug in TableWriteId's hashCode() implementation that is breaking
hashcode contract.

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 91 insertions(+), 39 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java:

http://gerrit.cloudera.org:8080/#/c/21087/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3694
PS2, Line 3694:     }
> I think, tests covered in test_events_custom_configs.py are enough for thes
I might miss something. Do we already have tests in test_events_custom_configs.py that will fail without this fix? Or do you mean the new tests added in another patch? https://gerrit.cloudera.org/c/21065/

The motivation of adding test coverage is that when we backport this to older branches, we can run tests to verify the backport is correct. This save some cost of review on the backports.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Feb 2024 23:48:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 10: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sun, 03 Mar 2024 07:02:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 4: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10329/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Feb 2024 22:52:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 5:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 16:36:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 9: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Mar 2024 16:27:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 28 Feb 2024 22:51:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

During AllocWriteIdEvent process, txnId to tableWriteIds mapping is
not added to catalog in the following cases:
1. When CREATE_TABLE event is followed by ALLOC_WRITE_ID_EVENT for
table in the same batch of MetastoreEventsProcessor.processEvents(),
process AllocWriteIdEvent do not add txnId to tableWriteIds mapping.
2. When table is not loaded.

This patch removes the usage of get table from catalog in the
event constructors. Also adds the txnId to tableWriteIds mapping in
catalog even when table is not loaded. And ensures the write ids
are not added to table if it is a non-partitioned table. Also fixed
a bug in TableWriteId's hashCode() implementation that is breaking
hashcode contract.

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 91 insertions(+), 39 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Feb 2024 17:17:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21087/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21087/4//COMMIT_MSG@11
PS4, Line 11: 1
Isn't 1 a sub case of 2? After getting a CREATE_TABLE the table shouldn't be loaded yet.


http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3937
PS4, Line 3937:     if (tbl.getNumClusteringCols() == 0) {
Can you add a comment with some explanation (or some pointer to an explanation) about why this is only needed for partitioned tables?


http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3938
PS4, Line 3938:       LOG.debug("Not adding write ids to table {}.{} for event {} since it is "
nit: missing "a"?


http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2824
PS4, Line 2824: msTbl_
Setting this member looks counter-intuitive to me in a property like function.

A function like init() or lookupTable() could be added that is called before calling in processIfEnabled before isEventProcessingDisabled() and is a NOOP for other event types.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 15:35:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

During AllocWriteIdEvent process, txnId to tableWriteIds mapping is
not added to catalog in the following cases:
1. When CREATE_TABLE event is followed by ALLOC_WRITE_ID_EVENT for
table in the same batch of MetastoreEventsProcessor.processEvents(),
process AllocWriteIdEvent do not add txnId to tableWriteIds mapping.
2. When table is not loaded.

This patch removes the usage of get table from catalog in the
event constructors. Also adds the txnId to tableWriteIds mapping in
catalog even when table is not loaded. And ensures the write ids
are not added to table if it is a non-partitioned table. Also fixed
a bug in TableWriteId's hashCode() implementation that is breaking
hashcode contract.

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 194 insertions(+), 39 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 6:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 17:12:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 196 insertions(+), 39 deletions(-)

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/21087/7
--
To view, visit http://gerrit.cloudera.org:8080/21087
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

During AllocWriteIdEvent process, txnId to tableWriteIds mapping is
not added to catalog in the following cases:
1. When CREATE_TABLE event is followed by ALLOC_WRITE_ID_EVENT for
the table in the same batch of MetastoreEventsProcessor.processEvents(),
process AllocWriteIdEvent cannot find catalog table since
CREATE_TABLE is not processed by the time of AllocWriteIdEvent
object construction.
2. When catalog table is present. But it is not loaded.

This patch removes the usage of get table from catalog in the
event constructors. Also adds the txnId to tableWriteIds mapping in
catalog even when table is not loaded. And ensures the write ids
are not added to table if it is a non-partitioned table. Also fixed
a bug in TableWriteId's hashCode() implementation that is breaking
hashcode contract.

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 196 insertions(+), 39 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.

Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 9: Code-Review+2

LGTM. Carry +1 from Csaba.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Mar 2024 21:25:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Feb 2024 03:17:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2824
PS4, Line 2824: msTbl_
> Setting this member looks counter-intuitive to me in a property like functi
oops, I see that https://gerrit.cloudera.org/#/c/21087/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2824 is about the same



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 15:36:23 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 6: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/10341/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 21:35:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Thu, 29 Feb 2024 17:01:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Quanlong Huang, Sai Hemanth Gantasala, Csaba Ringhofer, Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Testing:
- Added tests in MetastoreEventsProcessorTest

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 9:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Mar 2024 11:48:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.

Hello Impala Public Jenkins,

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

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

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

During AllocWriteIdEvent process, txnId to tableWriteIds mapping is
not added to catalog in the following cases:
1. When CREATE_TABLE event is followed by ALLOC_WRITE_ID_EVENT for
table in the same batch of MetastoreEventsProcessor.processEvents(),
process AllocWriteIdEvent do not add txnId to tableWriteIds mapping.
2. When table is not loaded.

This patch removes the usage of get table from catalog in the
event constructors. Also adds the txnId to tableWriteIds mapping in
catalog even when table is not loaded. And ensures the write ids
are not added to table if it is a non-partitioned table. Also fixed
a bug in TableWriteId's hashCode() implementation that is breaking
hashcode contract.

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
4 files changed, 110 insertions(+), 39 deletions(-)

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

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 6: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/21087/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21087/4//COMMIT_MSG@11
PS4, Line 11: 1
> In case-1, catalog table is not present because CREATE_TABLE event is not p
thanks, it is clear now!


http://gerrit.cloudera.org:8080/#/c/21087/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21087/6//COMMIT_MSG@17
PS6, Line 17: 
Can you mention reload event too? My understanding is that 1. was also relevant for it the patch fixed it.


http://gerrit.cloudera.org:8080/#/c/21087/6//COMMIT_MSG@21
PS6, Line 21: Also fixed
            : a bug in TableWriteId's hashCode() implementation that is breaking
            : hashcode contract.
Could this lead to correctness / perf issues?


http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java:

http://gerrit.cloudera.org:8080/#/c/21087/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2824
PS4, Line 2824:  can b
> Yes. Have added a TODO for now. This gerrit is required for https://gerrit.
ack



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Mar 2024 17:31:37 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21087 )

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................


Patch Set 10:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Mar 2024 21:26:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

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

Change subject: IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping
......................................................................

IMPALA-12851: Fix AllocWriteIdEvent process issue to add txnId-tableWriteIds mapping

Testing:
- Added tests in MetastoreEventsProcessorTest

Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Reviewed-on: http://gerrit.cloudera.org:8080/21087
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/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/TableWriteId.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
M fe/src/test/java/org/apache/impala/catalog/metastore/CatalogHmsSyncToLatestEventIdTest.java
5 files changed, 198 insertions(+), 42 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8b1a918befd4ee694880fd4e3cc04cb55b64955f
Gerrit-Change-Number: 21087
Gerrit-PatchSet: 11
Gerrit-Owner: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Anonymous Coward <k....@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Sai Hemanth Gantasala <sa...@cloudera.com>