You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2024/09/11 11:21:33 UTC
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21782
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
When debugging stale metadata, it'd be helpful to know what catalog
version of the tables are used and what's the time when catalogd loads
those versions. This patch exposes these info in the query profile for
each referenced table. E.g.
Table Versions: tpch.customer, 2249, 1726052668932, Wed Sep 11 19:04:28 CST 2024
tpch.nation, 2255, 1726052790140, Wed Sep 11 19:06:30 CST 2024
tpch.orders, 2257, 1726052803258, Wed Sep 11 19:06:43 CST 2024
tpch.lineitem, 2254, 1726052785384, Wed Sep 11 19:06:25 CST 2024
tpch.supplier, 2256, 1726052794235, Wed Sep 11 19:06:34 CST 2024
Each line consists of the table name, catalog version, loaded timestamp
and the timestamp string.
Implementation:
The loaded timestamp is updated whenever a CatalogObject updates its
catalog version in catalogd. It's passed to impalads with the
TCatalogObject broadcasted by statestore, or in DDL/DML responses.
Tests:
- Adds e2e test
Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
---
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/query_test/test_observability.py
M tests/shell/test_shell_commandline.py
22 files changed, 158 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/21782/1
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Jason Fehr, Csaba Ringhofer, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21782
to look at the new patch set (#3).
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
When debugging stale metadata, it'd be helpful to know what catalog
version of the tables are used and what's the time when catalogd loads
those versions. This patch exposes these info in the query profile for
each referenced table. E.g.
Table Versions: tpch.customer, 2249, 1726052668932, Wed Sep 11 19:04:28 CST 2024
tpch.nation, 2255, 1726052790140, Wed Sep 11 19:06:30 CST 2024
tpch.orders, 2257, 1726052803258, Wed Sep 11 19:06:43 CST 2024
tpch.lineitem, 2254, 1726052785384, Wed Sep 11 19:06:25 CST 2024
tpch.supplier, 2256, 1726052794235, Wed Sep 11 19:06:34 CST 2024
Each line consists of the table name, catalog version, loaded timestamp
and the timestamp string.
Implementation:
The loaded timestamp is updated whenever a CatalogObject updates its
catalog version in catalogd. It's passed to impalads with the
TCatalogObject broadcasted by statestore, or in DDL/DML responses.
Currently, the loaded timestamp is added for table, view, function, data
source, and hdfs cache pool in catalogd. However, only those of table
and view are applied used in impalad. For the loaded timestamp of other
types, users can check them in the /catalog WebUI of catalogd.
Tests:
- Adds e2e test
Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
---
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/query_test/test_observability.py
M tests/shell/test_shell_commandline.py
21 files changed, 155 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/21782/3
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java@349
PS3, Line 349: public long getCatalogVersion() { return 0; }
> Should this be done in CtasTargetTable?
I agree with this suggestion since the ctas target should always be a table of some sort that needs to be versioned by the catalog. Adding these functions in the parent class should result in new subclasses automatically getting this functionality.
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 23:21:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
When debugging stale metadata, it'd be helpful to know what catalog
version of the tables are used and what's the time when catalogd loads
those versions. This patch exposes these info in the query profile for
each referenced table. E.g.
Table Versions: tpch.customer, 2249, 1726052668932, Wed Sep 11 19:04:28 CST 2024
tpch.nation, 2255, 1726052790140, Wed Sep 11 19:06:30 CST 2024
tpch.orders, 2257, 1726052803258, Wed Sep 11 19:06:43 CST 2024
tpch.lineitem, 2254, 1726052785384, Wed Sep 11 19:06:25 CST 2024
tpch.supplier, 2256, 1726052794235, Wed Sep 11 19:06:34 CST 2024
Each line consists of the table name, catalog version, loaded timestamp
and the timestamp string.
Implementation:
The loaded timestamp is updated whenever a CatalogObject updates its
catalog version in catalogd. It's passed to impalads with the
TCatalogObject broadcasted by statestore, or in DDL/DML responses.
Tests:
- Adds e2e test
Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
---
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
M fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/query_test/test_observability.py
M tests/shell/test_shell_commandline.py
22 files changed, 160 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/21782/2
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
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/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
When debugging stale metadata, it'd be helpful to know what catalog
version of the tables are used and what's the time when catalogd loads
those versions. This patch exposes these info in the query profile for
each referenced table. E.g.
Original Table Versions: tpch.customer, 2249, 1726052668932, Wed Sep 11 19:04:28 CST 2024
tpch.nation, 2255, 1726052790140, Wed Sep 11 19:06:30 CST 2024
tpch.orders, 2257, 1726052803258, Wed Sep 11 19:06:43 CST 2024
tpch.lineitem, 2254, 1726052785384, Wed Sep 11 19:06:25 CST 2024
tpch.supplier, 2256, 1726052794235, Wed Sep 11 19:06:34 CST 2024
Each line consists of the table name, catalog version, loaded timestamp
and the timestamp string.
Implementation:
The loaded timestamp is updated whenever a CatalogObject updates its
catalog version in catalogd. It's passed to impalads with the
TCatalogObject broadcasted by statestore, or in DDL/DML responses.
Currently, the loaded timestamp is added for table, view, function, data
source, and hdfs cache pool in catalogd. However, only those of table
and view are applied used in impalad. For the loaded timestamp of other
types, users can check them in the /catalog WebUI of catalogd.
Tests:
- Adds e2e test
Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Reviewed-on: http://gerrit.cloudera.org:8080/21782
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/CtasTargetTable.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/VirtualTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/query_test/test_observability.py
M tests/shell/test_shell_commandline.py
20 files changed, 150 insertions(+), 14 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java@349
PS3, Line 349:
> I agree with this suggestion since the ctas target should always be a table
Moved this to CtasTargetTable but still keep returning 0 for simplicity.
This is generated by the Analyzer and doesn't exist in StmtTableCache since the table doesn't exist at the beginning. Showing the catalog version of the just-created target table needs additional code. I think it's not that useful since there are no concurrent INSERTs on the table. So we don't need to reason about what version of it is used.
BTW, if the target table already exists and we use CTAS with IF NOT EXISTS, StmtTableCache will be able to load the table so it appears in the profile as well. The INSERT part of the CTAS is skipped since the table already exists.
I change "Table Versions" to "Original Table Versions" to reflect that those are catalog versions before the statement is executed. We can add the catalog version of the target table after DDL/DML statements finish in future JIRAs.
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@170
PS3, Line 170:
> A VirtualTable by definition is not represented in HMS, and so wouldn't hav
Done. Moved these to VirtualTable. They are created by the Analyzer and don't exist in the StmtTableCache. So these functions are actually not used.
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2585
PS2, Line 2585: // Add the catalog versions and loaded timestamps.
> I'm not sure about the Thrift/JSON profile. If we cannot use different for
A table like ExecSummary adds additional padding. Keeps the current format since I think it's easier to parse and extract info from it.
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 09:04:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:30:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@170
PS3, Line 170: @Override
> I considered making the same suggestion, but I ended up deciding that I pre
A VirtualTable by definition is not represented in HMS, and so wouldn't have a catalog version or last loaded time.
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 23:30:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3:
(7 comments)
> The Catalog.java file sets the last modified time on db, table, view, function, data source, and hdfs cache pool. However, only the table and view are actually reported in the profile. Do we need to set last modified time on the other object types?
Thanks for the quick review!
Currently, only the timestamp of table/view are used in impalad so only those are applied. For other types users can check them in the /catalog WebUI of catalogd. We can apply them in impalad in the future e.g. if we find a place to show them in profiles. Updated the commit message about this.
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
File fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java@46
PS2, Line 46: public void setLastLoadedTimeMs(long timeMs) {
> Is this setter needed since mTimeMs_ is set in the "setCatalogVersion" func
Yes, we need it in ImpaladCatalog. When impalad applies the update from catalogd, it sets the catalog version and mTimeMs_ is updated as well. We need to reset the mTimeMs_ to the actually timestamp sent from catalogd:
https://gerrit.cloudera.org/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java#491
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java@70
PS2, Line 70:
> Nit: these two functions could be overridden in the parent IcebergDeleteTab
Good point! Done.
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java@66
PS2, Line 66:
> Nit: these two functions could be overridden in the parent IcebergDeleteTab
Done
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@346
PS2, Line 346: catalogObject.getCatalog_version(), catalogObject.getLast_modified_time_ms());
> Does last modified time also need to be set on db, function, data source, a
They are currently not used in impalad yet, e.g. not shown in the query profiles. I think it's OK to limit the scope of this change.
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@807
PS2, Line 807: resp.object_loaded_time_ms, new SqlConstraints(primaryKeys, foreignKeys),
> Does last modified time also need to be set on db, function, data source, a
Same as above, just set it for tables to be simple.
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2584
PS2, Line 2584:
> Nit: add a blank line before this line and start the comment with // Add th
Done
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2585
PS2, Line 2585: // Add the catalog versions and loaded timestamps.
> Can this profile text be a formatted table like ExecSummary is?
I hope it can be parsed by tools like Workload Management. Maybe the current form is easier for parsing. Or do we have a way to pretty print this only in the TEXT profile, and keep the THRIFT/JSON profile using this form?
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Sep 2024 23:06:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16930/ : 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/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Sep 2024 23:35:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16943/ : 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/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 09:32:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@170
PS3, Line 170: @Override
> Could these be done in VirtualTable to cover several subclasses?
I considered making the same suggestion, but I ended up deciding that I prefer not implementing these functions in VirtualTable. That way, if someone adds a new subclass, they are forced by the compiler to consider how their subclass should handle this functionality.
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 23:16:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3: -Code-Review
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 23:21:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Jason Fehr, Csaba Ringhofer, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/21782
to look at the new patch set (#4).
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
When debugging stale metadata, it'd be helpful to know what catalog
version of the tables are used and what's the time when catalogd loads
those versions. This patch exposes these info in the query profile for
each referenced table. E.g.
Original Table Versions: tpch.customer, 2249, 1726052668932, Wed Sep 11 19:04:28 CST 2024
tpch.nation, 2255, 1726052790140, Wed Sep 11 19:06:30 CST 2024
tpch.orders, 2257, 1726052803258, Wed Sep 11 19:06:43 CST 2024
tpch.lineitem, 2254, 1726052785384, Wed Sep 11 19:06:25 CST 2024
tpch.supplier, 2256, 1726052794235, Wed Sep 11 19:06:34 CST 2024
Each line consists of the table name, catalog version, loaded timestamp
and the timestamp string.
Implementation:
The loaded timestamp is updated whenever a CatalogObject updates its
catalog version in catalogd. It's passed to impalads with the
TCatalogObject broadcasted by statestore, or in DDL/DML responses.
Currently, the loaded timestamp is added for table, view, function, data
source, and hdfs cache pool in catalogd. However, only those of table
and view are applied used in impalad. For the loaded timestamp of other
types, users can check them in the /catalog WebUI of catalogd.
Tests:
- Adds e2e test
Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
---
M common/thrift/CatalogObjects.thrift
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/CtasTargetTable.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTimeTravelTable.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/VirtualTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/query_test/test_observability.py
M tests/shell/test_shell_commandline.py
20 files changed, 150 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/82/21782/4
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 2:
(7 comments)
The Catalog.java file sets the last modified time on db, table, view, function, data source, and hdfs cache pool. However, only the table and view are actually reported in the profile. Do we need to set last modified time on the other object types?
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
File fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java@46
PS2, Line 46: public void setLastLoadedTimeMs(long timeMs) {
Is this setter needed since mTimeMs_ is set in the "setCatalogVersion" function?
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java@70
PS2, Line 70: public long getCatalogVersion() { return 0; }
Nit: these two functions could be overridden in the parent IcebergDeleteTable.
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java@66
PS2, Line 66: public long getCatalogVersion() { return 0; }
Nit: these two functions could be overridden in the parent IcebergDeleteTable.
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@346
PS2, Line 346: catalogObject.getCatalog_version(), catalogObject.getLast_modified_time_ms());
Does last modified time also need to be set on db, function, data source, and hdfs cache pool like it is set on Catalog.getTCatalogObject?
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@807
PS2, Line 807: resp.object_loaded_time_ms, new SqlConstraints(primaryKeys, foreignKeys),
Does last modified time also need to be set on db, function, data source, and hdfs cache pool?
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2584
PS2, Line 2584: // Also adds the catalog versions and loaded timestamps.
Nit: add a blank line before this line and start the comment with // Add the catalog...
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2585
PS2, Line 2585: FrontendProfile.getCurrent().addInfoString("Table Versions",
Can this profile text be a formatted table like ExecSummary is?
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Sep 2024 15:37:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16920/ : 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/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Sep 2024 11:53:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 4: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@170
PS3, Line 170:
> Done. Moved these to VirtualTable. They are created by the Analyzer and don
Ah, I misunderstood the purpose of the VirtualTable class. Makes sense to me now to move these functions up to the VirtualTable class.
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2585
PS2, Line 2585: // Add the catalog versions and loaded timestamps.
> A table like ExecSummary adds additional padding. Keeps the current format
Sounds good to me.
Done
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 15:14:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10951/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:30:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/16921/ : 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/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Sep 2024 11:53:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10944/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 01:08:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Jason Fehr (Code Review)" <ge...@cloudera.org>.
Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3: Code-Review+1
(7 comments)
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
File fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java@46
PS2, Line 46: public void setLastLoadedTimeMs(long timeMs) {
> Yes, we need it in ImpaladCatalog. When impalad applies the update from cat
thanks for the explanation!
Done
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergEqualityDeleteTable.java@70
PS2, Line 70:
> Good point! Done.
Done
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java
File fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/IcebergPositionDeleteTable.java@66
PS2, Line 66:
> Done
Done
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@346
PS2, Line 346: catalogObject.getCatalog_version(), catalogObject.getLast_modified_time_ms());
> They are currently not used in impalad yet, e.g. not shown in the query pro
Done
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@807
PS2, Line 807: resp.object_loaded_time_ms, new SqlConstraints(primaryKeys, foreignKeys),
> Same as above, just set it for tables to be simple.
Done
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2584
PS2, Line 2584:
> Done
Done
http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2585
PS2, Line 2585: // Add the catalog versions and loaded timestamps.
> I hope it can be parsed by tools like Workload Management. Maybe the curren
I'm not sure about the Thrift/JSON profile. If we cannot use different formats, it's definitely better to be parseable in Thrift/JSON.
Workload management reads from the underlying ClientRequestState object that was built for each query. As long as this information is accessible there (easy to add if not), then it can be read by workload management.
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 19:15:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 16:30:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Sep 2024 21:51:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 06:17:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java@349
PS3, Line 349: public long getCatalogVersion() { return 0; }
Should this be done in CtasTargetTable?
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@170
PS3, Line 170: @Override
Could these be done in VirtualTable to cover several subclasses?
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 21:22:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles
......................................................................
Patch Set 4: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java@349
PS3, Line 349:
> Moved this to CtasTargetTable but still keep returning 0 for simplicity.
Ack
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java
File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java:
http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@170
PS3, Line 170:
> Ah, I misunderstood the purpose of the VirtualTable class. Makes sense to
Done
--
To view, visit http://gerrit.cloudera.org:8080/21782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98
Gerrit-Change-Number: 21782
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Sep 2024 17:24:36 +0000
Gerrit-HasComments: Yes