You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tianyi Wang (Code Review)" <ge...@cloudera.org> on 2018/02/27 01:13:43 UTC
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9458
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
IMPALA-6583: Wait for non-empty catalog update at impalad startup
Currently an empty statestore catalog update would enable impalad to
begin compiling queries. It breaks many custom cluster tests. This patch
makes impalad wait for a non-zero catalog version.
Testing: test_single_coordinator_cluster_config is run in a loop for
several hours on both 2.x and master branch. Core tests are also run.
Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
---
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
1 file changed, 6 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/9458/1
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9458/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/9458/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@213
PS2, Line 213: if (newCatalogVersion > INITIAL_CATALOG_VERSION) {
> This is subtle and needs an explanation. How can we get an update with INIT
I tested this and it seems the statestore sends out empty non-delta topic updates while the catalogd is starting up.
I think your fixes are correct, but we should explain with a comment here.
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Feb 2018 18:23:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has abandoned this change. ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2030/
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 28 Feb 2018 19:14:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
IMPALA-6583: Wait for non-empty catalog update at impalad startup
Currently an empty statestore catalog update would enable impalad to
begin compiling queries. It breaks many custom cluster tests. This patch
makes impalad wait for a non-zero catalog version.
Testing: test_single_coordinator_cluster_config is run in a loop for
several hours. Core tests are also run.
Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
3 files changed, 23 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/9458/4
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
IMPALA-6583: Wait for non-empty catalog update at impalad startup
Currently an empty statestore catalog update would enable impalad to
begin compiling queries. It breaks many custom cluster tests. This patch
makes impalad wait for a non-zero catalog version.
Testing: test_single_coordinator_cluster_config is run in a loop for
several hours on both 2.x and master branch. Core tests are also run.
Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
2 files changed, 8 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/9458/2
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
IMPALA-6583: Wait for non-empty catalog update at impalad startup
Currently an empty statestore catalog update would enable impalad to
begin compiling queries. It breaks many custom cluster tests. This patch
makes impalad wait for a non-zero catalog version.
Testing: test_single_coordinator_cluster_config is run in a loop for
several hours. Core tests are also run.
Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
3 files changed, 22 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/9458/3
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Tianyi Wang (Code Review)" <ge...@cloudera.org>.
Tianyi Wang has restored this change. ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
Restored
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: restore
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/9458/3/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/9458/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1123
PS3, Line 1123: // In case of an empty new catalog, the version should still change to reflect the
This is a more elaborate version of your previous comment :).
The important part to me is that Impalads wait for a catalog update with version > 0 before accepting requests.
http://gerrit.cloudera.org:8080/#/c/9458/3/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/9458/3/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@519
PS3, Line 519: public void setIsReady(boolean isReady) {
As far as I can tell, we can get rid of this function altogether.
http://gerrit.cloudera.org:8080/#/c/9458/3/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/9458/3/fe/src/main/java/org/apache/impala/service/Frontend.java@802
PS3, Line 802: * the first non-empty catalog update is received from the statestore.
... the first catalog update with a version > INITIAL_CATALOG_VERSION is received.
I think "non-empty" could be misleading because this logic will also work if the catalog is empty.
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 28 Feb 2018 17:12:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9458/3/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/9458/3/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@519
PS3, Line 519: public void setIsReady(boolean isReady) {
> BE has a InProcessImpalaServer, which calls setIsReady() and tests things w
I checked again and you are right. This is a really odd thing to do, but let's leave it for now.
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 28 Feb 2018 19:13:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/9458/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/9458/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1123
PS2, Line 1123: // The reset operation itself should bump the catalog version.
The important "why" part is missing from the comment. My understanding is that impalad catalogs become ready once the version is > 0, so we need to bump this version to ensure that impalads become ready even if the catalog is empty. Correct?
http://gerrit.cloudera.org:8080/#/c/9458/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java:
http://gerrit.cloudera.org:8080/#/c/9458/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@213
PS2, Line 213: if (newCatalogVersion > INITIAL_CATALOG_VERSION) {
This is subtle and needs an explanation. How can we get an update with INITIAL_CATALOG_VERSION?
From looking at CatalogServer::Start(), we see this sequence:
* JniCatalog is created which creates a CatalogServiceCatalog and calls reset() on that catalog. At this point the catalog is populated and should have a version > 0.
* After that first step, the catalog server registers with the statestore and starts processing catalog topic updates.
So how can an impalad ever see an update with a version of 0?
http://gerrit.cloudera.org:8080/#/c/9458/2/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java@214
PS2, Line 214: isReady_.set(true);
Do we still need this flag? It seems to be implied by version > 0.
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Feb 2018 18:08:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 28 Feb 2018 22:56:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6583: Wait for non-empty catalog update at impalad startup
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/9458 )
Change subject: IMPALA-6583: Wait for non-empty catalog update at impalad startup
......................................................................
IMPALA-6583: Wait for non-empty catalog update at impalad startup
Currently an empty statestore catalog update would enable impalad to
begin compiling queries. It breaks many custom cluster tests. This patch
makes impalad wait for a non-zero catalog version.
Testing: test_single_coordinator_cluster_config is run in a loop for
several hours. Core tests are also run.
Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Reviewed-on: http://gerrit.cloudera.org:8080/9458
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
3 files changed, 23 insertions(+), 13 deletions(-)
Approvals:
Alex Behm: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/9458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie5f09d03497543f283b39c6186ecfda2e0097c87
Gerrit-Change-Number: 9458
Gerrit-PatchSet: 5
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>