You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Alex Behm (Code Review)" <ge...@cloudera.org> on 2017/09/20 15:34:27 UTC
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Alex Behm has uploaded a new change for review.
http://gerrit.cloudera.org:8080/8112
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
This improves an existing workaround for HIVE-12730 to avoid
having the HMS wipe stats as a side-effect of an alterTable()
RPC. The new workaround uses DO_NOT_UPDATE_STATS which is
the recommended way of telling the HMS not to recompute/reset
statistics on its own.
Testing:
- core/hdfs run passed
Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 7 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/8112/1
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
Patch Set 1: -Code-Review
(2 comments)
Sorry, got a couple more questions after going through the code again. Will +1 again once I understand those scenarios.
http://gerrit.cloudera.org:8080/#/c/8112/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 800: partition.putToParameters(MetastoreShim.statsGeneratedViaStatsTaskParam());
Are partition level stats also affected, just wanted to make sure that we don't need to set it here as well.
PS1, Line 822: Pair<String, String> statsTaskParam = MetastoreShim.statsGeneratedViaStatsTaskParam();
: msTbl.putToParameters(statsTaskParam.first, statsTaskParam.second);
This looks redundant and is overwritten anyway right?
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
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/8112 )
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
This improves an existing workaround for HIVE-12730 to avoid
having the HMS wipe stats as a side-effect of an alterTable()
RPC. The new workaround uses DO_NOT_UPDATE_STATS which is
the recommended way of telling the HMS not to recompute/reset
statistics on its own.
Testing:
- core/hdfs run passed
Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Reviewed-on: http://gerrit.cloudera.org:8080/8112
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 9 insertions(+), 6 deletions(-)
Approvals:
Alex Behm: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8112 )
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1256/
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 21 Sep 2017 20:47:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change. ( http://gerrit.cloudera.org:8080/8112 )
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Sep 2017 18:00:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has uploaded a new patch set (#2).
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
This improves an existing workaround for HIVE-12730 to avoid
having the HMS wipe stats as a side-effect of an alterTable()
RPC. The new workaround uses DO_NOT_UPDATE_STATS which is
the recommended way of telling the HMS not to recompute/reset
statistics on its own.
Testing:
- core/hdfs run passed
Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
1 file changed, 9 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/8112/2
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8112 )
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 22 Sep 2017 00:53:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
Patch Set 1: Code-Review+1
Should we add more alter scenarios to "alter-table.test" to make sure alterTable() doesn't update stats?
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8112 )
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-Change-Number: 8112
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Sep 2017 20:47:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8112/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 800: partition.putToParameters(MetastoreShim.statsGeneratedViaStatsTaskParam());
> Are partition level stats also affected, just wanted to make sure that we d
I checked the Hive/HMS code carefully and partition-level stat are not affected. Feel free to look yourself. The problematic stats-wiping function is MetastoreUtils.updateTableStatsFast().
While at some level I'd love to pass DO_NOT_UPDATE_STATS in all RPC to the HMS, I feel like being too defensive is confusing to readers. Adding the flag here will actually add the table property to the partition which is also weird.
PS1, Line 822: Pair<String, String> statsTaskParam = MetastoreShim.statsGeneratedViaStatsTaskParam();
: msTbl.putToParameters(statsTaskParam.first, statsTaskParam.second);
> This looks redundant and is overwritten anyway right?
Setting this flag is required for for stats changes to take effect (table or partition). Why do you way this is redundant? It is different than the DO_NOT_UPDATE_STATS flag.
Added comments.
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5954: Set DO NOT UPDATE STATS in alterTable() RPCs to HMS.
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-5954: Set DO_NOT_UPDATE_STATS in alterTable() RPCs to HMS.
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8112/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
Line 800: // HMS requires this param for stats changes to take effect.
> I checked the Hive/HMS code carefully and partition-level stat are not affe
Thanks for explaining. I agree.
PS1, Line 822: }
: // HMS requires this param for stats changes to take effect.
> Setting this flag is required for for stats changes to take effect (table o
Sorry, I got confused. I now see how all this works.
--
To view, visit http://gerrit.cloudera.org:8080/8112
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I05c63315579d66efeaa4c65105372ae68820ab2f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes