You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org> on 2023/05/03 19:40:11 UTC
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Sai Hemanth Gantasala has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19838
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_whitelist' which can be used to
define what table properties need file metadata to be reloaded. If this
value is set to empty, this optimization is not in effect and the file
metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 161 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/1
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 1
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 13:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/19838/11/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/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1469
PS11, Line 1469: if (canBeSkipped()) {
> This also checks tblproperties but acts like blacklist (i.e. skip if match)
This skips the processing alter table event itself. In this propose to skipFilemetadata reload only. So I think we should keep this.
http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1537
PS11, Line 1537: infoLog("Change in number of columns detected for table {}.{} from {} to {}",
> nit: let's try to log the numbers. Also use fully qualified table name to e
Ack
http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1546
PS11, Line 1546: infoLog("Change in table schema detected for table {}.{} from {} to {} ",
> nit: let's try to log the new and old name and type of this column
Ack
http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1555
PS11, Line 1555:
> nit: let's try to log the new and old owners, e.g.
Ack
http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1568
PS11, Line 1568: org.apache.hadoop.hive.metastore.api.Table afterTable) {
> nit: let's log the new and old strings
Ack
http://gerrit.cloudera.org:8080/#/c/19838/11/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/19838/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2813
PS11, Line 2813: alterTableSetOwner(testTblNa
> Shouldn't we do this in Hive?
Currently, there is no method to change the owner from HMS. I have now added a method to change it from HMS.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 13
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jun 2023 21:07:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13107/ : 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/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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: Tue, 23 May 2023 20:33:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13216/ : 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/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 8
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 03:32:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#8).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 217 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/8
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 8
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "configuration is used to whitelist the table properti
> Ok then, so we don't need to include 'transactional' in the whitelist confi
Though this through once more. I think that it would be better to include these properties "just in case" - I don't think that this has a performance impact as these rarely if ever change and we may be able to avoid some bugs with including them. I was thinking about the following 3:
- transactional
- transactional_properties
- TRANSLATED_TO_EXTERNAL
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 8
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jun 2023 12:03:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 10:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/13240/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 10
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jun 2023 04:21:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#14).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 241 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/14
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 14
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#2).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 164 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/2
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 2
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12923/ : 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/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 1
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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: Wed, 03 May 2023 20:02:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#5).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 196 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/5
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 5
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#12).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 241 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/12
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 12
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 17: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 17
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Jun 2023 09:47:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 18: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 18
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Jun 2023 09:47:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "configuration is used to whitelist the table properti
> About change of 'transactional': there is a test where it changes, though '
Ok then, so we don't need to include 'transactional' in the whitelist config.
For, transactional_properties, changing between insert_only and default would change the table metadata. File metadata will not be changed since the table is being translated from managed to external, so file metadata will not be modified.
http://gerrit.cloudera.org:8080/#/c/19838/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/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@374
PS4, Line 374: String whitelist = Backen
> Iceberg also use table property 'metadata_location' to denote the current s
Ack. Good point!!
http://gerrit.cloudera.org:8080/#/c/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@377
PS4, Line 377:
> My impression is that table property names are also case sensitive: I was a
Hmm so, do you think we should add 'EXTERNAL' also to the whitelist config?
http://gerrit.cloudera.org:8080/#/c/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3936
PS4, Line 3936: for (String whitelistConfig : whitelistedTblProperties_) {
: String configBefore = beforeTable.getParameters().get(whitelistConfig);
: String configAfter = afterTable.getParameters().get(whitelistConfig);
: if (!Objects.equals(configBefore, configAfter)) return true;
: }
: return false;
: }
: }
:
> Can it be replaced with:
Ack
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 6
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 May 2023 16:37:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#3).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 201 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/3
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 3
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19838/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/19838/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2824
PS2, Line 2824: assertEquals(fileMetadataLoadAfter, fileMetadataLoadBefore);
> It would be great to test that some table property changes actually lead to
Ack
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 3
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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, 19 May 2023 00:08:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 11:
(6 comments)
LGTM. Just have some suggestions in the logging since we already have logs on the full table name for events. It'd be nice to add some more details about the ALTER_TABLE changes so admins don't need to check Hive's NOTIFICATION_LOGS.
http://gerrit.cloudera.org:8080/#/c/19838/11/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/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1469
PS11, Line 1469: if (canBeSkipped()) {
This also checks tblproperties but acts like blacklist (i.e. skip if match). Should we keep or remove it?
http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1537
PS11, Line 1537: infoLog("Change in number of columns detected for table {} in the database {} ",
nit: let's try to log the numbers. Also use fully qualified table name to ease grep commands. E.g.
Change in number of columns detected for table {}.{} from {} to {}
http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1546
PS11, Line 1546: infoLog("Change in table schema detected for table {} in the database {} ",
nit: let's try to log the new and old name and type of this column
http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1555
PS11, Line 1555: infoLog("Change in Ownership detected for table {} in the database {} ", tblName_,
nit: let's try to log the new and old owners, e.g.
Change in Ownership detected. Table: {}.{}, oldOwner: {}, newOwner: {}
http://gerrit.cloudera.org:8080/#/c/19838/11/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1568
PS11, Line 1568: infoLog("Change in whitelisted table properties detected for table {} in the" +
nit: let's log the new and old strings
http://gerrit.cloudera.org:8080/#/c/19838/11/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/19838/11/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2813
PS11, Line 2813: alterTableSetOwnerFromImpala
Shouldn't we do this in Hive?
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 11
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jun 2023 01:46:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13164/ : 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/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Jun 2023 17:36:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#9).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 218 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/9
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 9
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 18:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9420/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 18
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Jun 2023 09:47:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 3:
(6 comments)
Thanks for the changes, looks good! Few more comments.
http://gerrit.cloudera.org:8080/#/c/19838/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19838/3//COMMIT_MSG@17
PS3, Line 17: If any of these are changed, the file metadata reloading
: can be skipped.
What happens with the table property that was changed? While the table metadata is not reloaded, we still update the given properties, right?
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "to whitelist the table properties that are supposed t
> Actually both the compaction-related properties are not required, since hiv
Shouldn't we keep transactional and add transactional_properties?
I think that adding properties that rarely change is ok, even if it is not needed actually. The critical part is to avoid reload on properties that can change often, e.g. on every insert.
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "to whitelist the table properties that are supposed to refresh file metadata"
: "when these properties are changed.");
> I see that there are some query options that can be modified at run time. B
I didn't mean to use a query option here.
The idea is to have a special table property name that is not used for anything else than force reloading a table. This could act as a "safety valve" for users to cause reloading a table (without altering its semantics) even with default flags.
http://gerrit.cloudera.org:8080/#/c/19838/3/common/thrift/BackendGflags.thrift
File common/thrift/BackendGflags.thrift:
http://gerrit.cloudera.org:8080/#/c/19838/3/common/thrift/BackendGflags.thrift@257
PS3, Line 257:
nit: extra line
http://gerrit.cloudera.org:8080/#/c/19838/2/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/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3900
PS2, Line 3900: return false;
> If the whitelisted config is empty, we'll never skip file reload. The reaso
I see, agree with handling it like this.
Can you add a comment about it here and also mention this in the flag's description in catalog-server.cc ?
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3904
PS2, Line 3904: if (isFieldSchemaChanged(beforeTable, afterTable) ||
: !beforeTable.getOwner().equals(afterTable.getOwner()) ||
: !isCustomTblPropsChanged(beforeTable, afterTable)) {
: return true;
: }
> if any of the conditions are true then we can skip reloading file metadata.
So if one alter table event changes the field schema or the owner, then it is guaranteed that the table properties can not change in this event? If this is the case, then the first two checks seem redundant to me, as isCustomTblPropsChanged would also return false in this case because no table properties were changed at all.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 3
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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, 19 May 2023 08:05:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#7).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 196 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/7
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 11:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "transactional, transactional_properties, TRANSLATED_T
> Though this through once more. I think that it would be better to include t
Ack
http://gerrit.cloudera.org:8080/#/c/19838/8/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/19838/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@755
PS8, Line 755: skipFileMetadataReload_ =
> naming: maybe canSkipFileMetadataReload_ or simply skipFileMetadataReload_
Ack
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 11
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jun 2023 05:51:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#16).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 241 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/16
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 16
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 18: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 18
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Jun 2023 15:15:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 1:
(11 comments)
Thanks for making this change!
http://gerrit.cloudera.org:8080/#/c/19838/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19838/1//COMMIT_MSG@19
PS1, Line 19: file_metadata_reload_whitelist
nit: can we use a name indicating this is a list of table properties, e.g. file_metadata_reload_properties?
http://gerrit.cloudera.org:8080/#/c/19838/1//COMMIT_MSG@25
PS1, Line 25: statements the file metadata isn't reloaded.
Could you add a custom cluster test for the new flag?
http://gerrit.cloudera.org:8080/#/c/19838/1/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/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2569
PS1, Line 2569: null
nit: please also comment this with /*partitionToEventId*/
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3881
PS1, Line 3881: statement
nit: event
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3894
PS1, Line 3894: isFieldSchemaChanged(beforeTable, afterTable) ||
: !beforeTable.getOwner().equals(afterTable.getOwner())
Shouldn't we check these two before checking the whitelist table properties? Or is it on purpose to let the empty whitelist disable this optimization?
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3897
PS1, Line 3897: return true;
Should we check if there are other changes between "beforeTable" and "afterTable"? Is it possible that a HMS client changes the table location as well as table schema in one RPC (so generate one ALTER event)?
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3907
PS1, Line 3907: //check if columns are added or removed
: if (beforeCols.size() != afterCols.size())
: return true;
nit: needs a space after "//". Needs brackets for the if-clause based on our code style.
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3912
PS1, Line 3912: for (int i=0;i<beforeCols.size();i++) {
nit: need spaces
for (int i = 0; i < beforeCols.size(); i++) {
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3928
PS1, Line 3928: whitelistConfigs
Please make 'whitelistConfigs' a constant field so we don't need to parse the whilelist string for every event.
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3932
PS1, Line 3932: afterTable.getParameters().containsKey(whitelistConfig)) ||
Are we missing the case of "before && !after", i.e. the property is removed?
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3944
PS1, Line 3944:
nit: remove blank lines
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 1
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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, 04 May 2023 02:34:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 2:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "compactorthreshold.hive.compactor.delta.num.threshold
Why should Impala reload the table when these compactor props change?
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "compactorthreshold.hive.compactor.delta.num.threshold, "
: "compactorthreshold.hive.compactor.delta.pct.threshold, transactional",
We could add an Impala specific property here to give users the ability to force reload (without restarting the cluster and changing flags or messing with flags that have other uses). It could be also useful in tests.
For example:
impala.metadata_reload_prop
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@139
PS2, Line 139: "are supposed to reload file metadata these properties are changed.");
nit: missing "when"?
http://gerrit.cloudera.org:8080/#/c/19838/2/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/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@326
PS2, Line 326: ;
nit: members postfixed with _
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3891
PS2, Line 3891: realoding
typo
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3896
PS2, Line 3896: isSkipFileMetadataReload
naming: maybe canSkipFileMetadataReload would be clearer?
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3900
PS2, Line 3900: return false;
The return value is not clear to me:
does this mean that we should never skip reload is whitelistedTblProperties is empty or that we should always skip it?
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3904
PS2, Line 3904: if (isFieldSchemaChanged(beforeTable, afterTable) ||
: !beforeTable.getOwner().equals(afterTable.getOwner()) ||
: !isCustomTblPropsChanged(beforeTable, afterTable)) {
: return true;
: }
again, I don't understand the return value - if any of the conditions above is true, doesn't it mean that we have to reload the table?
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3942
PS2, Line 3942: equalsIgnoreCase
I am not 100% sure that we can ignore the case. I can imagine a properties like paths or base64 encoded values where the case does matter.
http://gerrit.cloudera.org:8080/#/c/19838/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/19838/2/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@2824
PS2, Line 2824: assertEquals(fileMetadataLoadAfter, fileMetadataLoadBefore);
It would be great to test that some table property changes actually lead to metadata load. For example the property I recommended in catalog-server.cc could be used to test that:
- adding leads to reload
- removing leads to reload
- changing leads to reload
- setting to the same value doesn't lead to reload
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 2
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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, 12 May 2023 10:52:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "to whitelist the table properties that are supposed t
> HiveMetaStore doesn't allow transactional and transactional_properties to b
About change of 'transactional': there is a test where it changes, though 'external' also changes in that case, so it is not need to look specifically for transactional:
https://github.com/apache/impala/blob/dc63ae514a445e3f197cab405b01a30c58015695/testdata/workloads/functional-query/queries/QueryTest/acid.test#L49
I was also able to change transactional_properties in Hive from 'insert_only' to 'default'
http://gerrit.cloudera.org:8080/#/c/19838/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/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@377
PS4, Line 377: toLowerCase
My impression is that table property names are also case sensitive: I was able to set a table property 'external', but and created a separate property from the default 'EXTERNAL'. So the 'external' in the flag and tests should be also all capital. (this was surprising to me, it absolutely doesn't make sense to me to use properties with only differences in case)
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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: Wed, 24 May 2023 07:13:44 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 12:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/13293/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 12
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jun 2023 20:19:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19838/9/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/19838/9/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1520
PS9, Line 1520: // There are lot of other alter statements which doesn't require file metadata reload
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 9
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jun 2023 03:59:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 7:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/19838/7/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/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@376
PS7, Line 376: db
nit: Just confused, what does "db" stands for here? I thought it's "database" at the first glance.. Will "key" or "k" looks better?
http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3908
PS7, Line 3908: !isCustomTblPropsChanged(beforeTable, afterTable)) {
For supportability, we'd better split these to seperate if-clause and add logs like "detected schema changed", "detected owner changed" when the conditions match. So in production we can reason about the behavior by logs.
Also, can we move these new methods to MetastoreEvents.java? So the logs can be tagged with event ids. It seems they don't need any private fields of CatalogServiceCatalog.
http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3926
PS7, Line 3926: !beforeCols.get(i).getType().equals(afterCols.get(i).getType()))
Need logs for these as well.
http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3939
PS7, Line 3939: if (!Objects.equals(configBefore, configAfter)) return true;
Need logs for this as well.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Jun 2023 13:36:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 3:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "to whitelist the table properties that are supposed t
> Why should Impala reload the table when these compactor props change?
Actually both the compaction-related properties are not required, since hive is now throwing "commit_compaction _event" for any change after compaction.
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "to whitelist the table properties that are supposed to refresh file metadata"
: "when these properties are changed.");
> We could add an Impala specific property here to give users the ability to
I see that there are some query options that can be modified at run time. But I cannot find an example in catalogD configs that can be modified at runtime. Can you please give some pointers on how to achieve this?
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@139
PS2, Line 139: DECLARE_string(state_store_host);
> nit: missing "when"?
Ack
http://gerrit.cloudera.org:8080/#/c/19838/2/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/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@326
PS2, Line 326: _
> nit: members postfixed with _
Ack
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3891
PS2, Line 3891: reloading
> typo
Ack
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3896
PS2, Line 3896: canSkipFileMetadataReloa
> naming: maybe canSkipFileMetadataReload would be clearer?
Ack
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3900
PS2, Line 3900: return false;
> The return value is not clear to me:
If the whitelisted config is empty, we'll never skip file reload. The reason I'm doing this is because if something unexpected happens in the customer's env, clearing this config should work the way it is working currently.
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3904
PS2, Line 3904: if (isFieldSchemaChanged(beforeTable, afterTable) ||
: !beforeTable.getOwner().equals(afterTable.getOwner()) ||
: !isCustomTblPropsChanged(beforeTable, afterTable)) {
: return true;
: }
> again, I don't understand the return value - if any of the conditions above
if any of the conditions are true then we can skip reloading file metadata. The above situations never change file metadata, only table metadata is changed.
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3942
PS2, Line 3942: equals(afterTabl
> I am not 100% sure that we can ignore the case. I can imagine a properties
Ack
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 3
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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, 19 May 2023 00:07:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 5:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/13129/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 5
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 25 May 2023 17:34:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 15: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9401/
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 15
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 05:51:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 14:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19838/13/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/19838/13/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1573
PS13, Line 1573: infoLog("Change in whitelisted table properties detected for table {}.{} " +
> nit: please also add 'whitelistConfig'
Ack
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 14
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jun 2023 22:04:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 16:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/13353/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 16
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 21 Jun 2023 02:16:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 7:
Did you run the tests with the newest changes? They still use lower case "external" so I think that there should be some failure.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Jun 2023 10:17:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12969/ : 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/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 2
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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: Tue, 09 May 2023 01:13:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 2:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/19838/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19838/1//COMMIT_MSG@19
PS1, Line 19: file_metadata_reload_propertie
> nit: can we use a name indicating this is a list of table properties, e.g.
Ack
http://gerrit.cloudera.org:8080/#/c/19838/1//COMMIT_MSG@25
PS1, Line 25: statements the file metadata isn't reloaded.
> Could you add a custom cluster test for the new flag?
Currently, there is no end-to-end test that verifies whether file metadata is reloaded or not. Can you suggest how to achieve this?
http://gerrit.cloudera.org:8080/#/c/19838/1/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/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2569
PS1, Line 2569:
> nit: please also comment this with /*partitionToEventId*/
Ack
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3881
PS1, Line 3881: }
> nit: event
Done
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3894
PS1, Line 3894: nt.
: */
> Shouldn't we check these two before checking the whitelist table properties
I would like to disable the optimization by setting the above flag to empty in case something works unexcepted. What do you think?
I can update the description of the config flag to explain the same.
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3897
PS1, Line 3897: org.apache.hadoop.hive.metastore.api.Table beforeTable,
> Should we check if there are other changes between "beforeTable" and "after
AFAIK, you can only change one specific thing (either serde or location or tbl properties etc.) on an alter event. For location change, we'll have to reload file metadata, which we are anyway doing now since we'll be returning false in that condition.
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3907
PS1, Line 3907: return true;
: }
: return false;
> nit: needs a space after "//". Needs brackets for the if-clause based on ou
Done
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3912
PS1, Line 3912: private boolean isFieldSchemaChanged(
> nit: need spaces
Done
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3928
PS1, Line 3928:
> Please make 'whitelistConfigs' a constant field so we don't need to parse t
Ack
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3932
PS1, Line 3932: org.apache.hadoop.hive.metastore.api.Table beforeTable,
> Are we missing the case of "before && !after", i.e. the property is removed
Ack
http://gerrit.cloudera.org:8080/#/c/19838/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3944
PS1, Line 3944: }
> nit: remove blank lines
Ack
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 2
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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: Tue, 09 May 2023 00:52:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13082/ : 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/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 3
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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, 19 May 2023 00:28:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 15:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9401/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 15
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 00:33:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 15: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 15
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 00:33:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19838/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/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@374
PS4, Line 374: whitelistedTblProperties_
Iceberg also use table property 'metadata_location' to denote the current snapshot file. If it changes we need to do a full table reload.
http://gerrit.cloudera.org:8080/#/c/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3936
PS4, Line 3936: if ((!beforeTable.getParameters().containsKey(whitelistConfig) &&
: afterTable.getParameters().containsKey(whitelistConfig)) ||
: (beforeTable.getParameters().containsKey(whitelistConfig) &&
: !afterTable.getParameters().containsKey(whitelistConfig)) ||
: (beforeTable.getParameters().containsKey(whitelistConfig) &&
: afterTable.getParameters().containsKey(whitelistConfig) &&
: !beforeTable.getParameters().get(whitelistConfig)
: .equals(afterTable.getParameters().get(whitelistConfig)))) {
: return true;
Can it be replaced with:
String cfgBefore = beforeTable.getParameters().get(whitelistConfig);
String cfgAfter = afterTable.getParameters().get(whitelistConfig));
if (!Objects.equals(cfgBefore, cfgAfter)) return true;
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 24 May 2023 09:47:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 14:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/13295/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 14
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jun 2023 22:26:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 14: Code-Review+2
Carrying Csaba's +1.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 14
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Jun 2023 00:32:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#4).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 202 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/4
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19838/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/19838/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@377
PS4, Line 377:
> Hmm so, do you think we should add 'EXTERNAL' also to the whitelist config?
yes, it should be "EXTERNAL" instead of "external"
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 6
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Jun 2023 09:22:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 8:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19838/8/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/19838/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1520
PS8, Line 1520: // There are lot of other alter statements which doesn't require file metadata reload
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 8
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 03:12:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 7:
Csaba, Zoltan, Quanlong -- Can you please do another round of review on my patch?
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Jun 2023 20:12:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#10).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 218 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/10
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 10
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/19838/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19838/3//COMMIT_MSG@17
PS3, Line 17: If any of these are changed, the file metadata reloading
: can be skipped.
> What happens with the table property that was changed? While the table meta
We will always reload table metadata along with the updated table properties. With this patch, we only intend to skip file metadata of the given table.
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc
File be/src/catalog/catalog-server.cc:
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "to whitelist the table properties that are supposed t
> Shouldn't we keep transactional and add transactional_properties?
HiveMetaStore doesn't allow transactional and transactional_properties to be changed. So these properties are not really needed. The thing is we don't know what properties that change often. I think it is upto the admin to configure these values.
Insert statement generates insert event which is already handled by the event processor.
http://gerrit.cloudera.org:8080/#/c/19838/2/be/src/catalog/catalog-server.cc@136
PS2, Line 136: "to whitelist the table properties that are supposed to refresh file metadata"
: "when these properties are changed. To skip this optimization, set the
> I didn't mean to use a query option here.
Ack
http://gerrit.cloudera.org:8080/#/c/19838/3/common/thrift/BackendGflags.thrift
File common/thrift/BackendGflags.thrift:
http://gerrit.cloudera.org:8080/#/c/19838/3/common/thrift/BackendGflags.thrift@257
PS3, Line 257: }
> nit: extra line
Ack
http://gerrit.cloudera.org:8080/#/c/19838/2/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/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3900
PS2, Line 3900: if (whitelisted
> I see, agree with handling it like this.
Ack
http://gerrit.cloudera.org:8080/#/c/19838/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3904
PS2, Line 3904: // but these are the most common types for alter statements.
: if (isFieldSchemaChanged(beforeTable, afterTable) ||
: !beforeTable.getOwner().equals(afterTable.getOwner()) ||
: !isCustomTblPropsChanged(beforeTable, afterTable)) {
:
> So if one alter table event changes the field schema or the owner, then it
So in one alter table query, either a table schema can be changed, or a table owner can be changed, or any table property (other whitelisted ones) can be changed, so if anyone of these changes, we can skip file metadata reloading.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 4
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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: Tue, 23 May 2023 20:12:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19838
to look at the new patch set (#6).
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 196 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/19838/6
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 6
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/13150/ : 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/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 6
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 May 2023 16:58:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 13: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19838/13/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/19838/13/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1573
PS13, Line 1573: infoLog("Change in whitelisted table properties detected for table {}.{} " +
nit: please also add 'whitelistConfig'
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 13
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Jun 2023 21:15:11 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
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/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
Reloading file metadata for medium to wide tables is heavy weight
operation in general. So it would be ideal from event processor
perspective to minimize file metadata reloading especially for
ALTER_TABLE statements which are quite common in metastore events.
This patch implements the above optimization by looking at before
and after table objects of an alter event and see if it corresponds
to ALTER TABLE add/change/replace column, set owner, set table
properties. If any of these are changed, the file metadata reloading
can be skipped. For inter-operability purpose this patch introduced a
new start-up flag 'file_metadata_reload_properties' which can be used
to define what table properties need file metadata to be reloaded. If
this value is set to empty, this optimization is not in effect and the
file metadata is always reloaded.
Testing: Added a unit test to confirm that, for certain alter table
statements the file metadata isn't reloaded.
Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Reviewed-on: http://gerrit.cloudera.org:8080/19838
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/catalog/catalog-server.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
8 files changed, 241 insertions(+), 12 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 19
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Sai Hemanth Gantasala (Code Review)" <ge...@cloudera.org>.
Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 7:
(4 comments)
> Patch Set 7:
>
> Did you run the tests with the newest changes? They still use lower case "external" so I think that there should be some failure.
Verified and addressed.
http://gerrit.cloudera.org:8080/#/c/19838/7/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/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@376
PS7, Line 376: db
> nit: Just confused, what does "db" stands for here? I thought it's "databas
Will do.
http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3908
PS7, Line 3908: !isCustomTblPropsChanged(beforeTable, afterTable)) {
> For supportability, we'd better split these to seperate if-clause and add l
Ack
http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3926
PS7, Line 3926: !beforeCols.get(i).getType().equals(afterCols.get(i).getType()))
> Need logs for these as well.
Ack
http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3939
PS7, Line 3939: if (!Objects.equals(configBefore, configAfter)) return true;
> Need logs for this as well.
Ack
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 7
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jun 2023 03:12:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 8: Code-Review+1
(1 comment)
just a naming suggestion, lgtm otherwise
http://gerrit.cloudera.org:8080/#/c/19838/8/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/19838/8/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@755
PS8, Line 755: isSkipFileMetadataReload_
naming: maybe canSkipFileMetadataReload_ or simply skipFileMetadataReload_ would be clearer?
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 8
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Jun 2023 10:04:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 9:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/13239/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 9
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jun 2023 04:19:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11534: Skip reloading file metadata for some ALTER TABLE events
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events
......................................................................
Patch Set 11: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19838
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6
Gerrit-Change-Number: 19838
Gerrit-PatchSet: 11
Gerrit-Owner: Sai Hemanth Gantasala <sa...@cloudera.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-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Jun 2023 06:53:13 +0000
Gerrit-HasComments: No