[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Thu, 05 Dec 2019 03:25:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. IMPALA-9211: Fix adding new table to stale db due to concurrent reset When adding a new table to the catalog, we first get the db object from dbCache and then add the table into it. When doing reset(), i.e. global INVALIDATE METADATA, we replace the whole dbCache with a new one and load db and table names from HMS. These two operations have race conflicts so should both be protected by a exclusive lock, i.e. write lock of version lock. Currently, CatalogServiceCatalog.addTable() does not get the db object within the write lock, which may get a stale db object and add new table into it. This patch moves the operations into the protection of write lock. Tests: - Ran test_concurrent_ddls.py without errors for CreateTable like IMPALA-9135 found. - Ran Core tests Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Reviewed-on: http://gerrit.cloudera.org:8080/14820 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java 2 files changed, 14 insertions(+), 11 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5312/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 04 Dec 2019 22:59:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 04 Dec 2019 22:59:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 2: Thank Csaba for your review! -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 04 Dec 2019 22:58:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 04 Dec 2019 16:48:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5186/ : 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/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 04 Dec 2019 00:55:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Hello Vihang Karajgaonkar, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14820 to look at the new patch set (#2). Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. IMPALA-9211: Fix adding new table to stale db due to concurrent reset When adding a new table to the catalog, we first get the db object from dbCache and then add the table into it. When doing reset(), i.e. global INVALIDATE METADATA, we replace the whole dbCache with a new one and load db and table names from HMS. These two operations have race conflicts so should both be protected by a exclusive lock, i.e. write lock of version lock. Currently, CatalogServiceCatalog.addTable() does not get the db object within the write lock, which may get a stale db object and add new table into it. This patch moves the operations into the protection of write lock. Tests: - Ran test_concurrent_ddls.py without errors for CreateTable like IMPALA-9135 found. - Ran Core tests Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java 2 files changed, 14 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/14820/2 -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 1: (1 comment) > Patch Set 1: Code-Review+1 > > (1 comment) http://gerrit.cloudera.org:8080/#/c/14820/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/14820/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1737 PS1, Line 1737: return db.getTable(tblName); > Hmm, I looked at the LocalDb implementation by mistake which does load the Ah yes, the comment is out-of-date. Removed it and renamed this function. BTW, LocalDb is only used in Coordinator in Local-Catalog mode (catalog-v2). For catalogd, all the catalog objects are still using the legacy implementation. -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 04 Dec 2019 00:26:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/14820/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/14820/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1737 PS1, Line 1737: return db.getTable(tblName); > Isn't this a lightweight op? https://github.com/apache/impala/blob/b421741d Hmm, I looked at the LocalDb implementation by mistake which does load the table if needed: https://github.com/apache/impala/blob/b421741d7f17868958c50b4b19f22d255071472a/fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java#L118 What made me assume that it is loaded is the comment for the function, which says "loading the metadata if needed.", but it seems that actually neither the db or the table is loaded. Can you fix the comment? It also makes sense to me to rename this function to "addIncompleteTable" -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 03 Dec 2019 17:28:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14820/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/14820/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1737 PS1, Line 1737: return db.getTable(tblName); > Is is necessary to keep the lock during getTable() too? The name is a bit m Isn't this a lightweight op? https://github.com/apache/impala/blob/b421741d7f17868958c50b4b19f22d255071472a/fe/src/main/java/org/apache/impala/catalog/Db.java#L183 I think it won't trigger any external RPCs, or do I miss anything? -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 03 Dec 2019 15:30:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14820/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/14820/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1737 PS1, Line 1737: return db.getTable(tblName); Is is necessary to keep the lock during getTable() too? The name is a bit misleading, as it also loads the table if it is an IncompleteTable (which is true in this case). The RPC to HMS makes it a potentially expensive call, and it would be better to avoid locking the whole catalog during the call. -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 03 Dec 2019 14:57:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Dec 2019 09:16:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5306/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Dec 2019 04:51:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14820 ) Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5183/ : 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/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Dec 2019 04:33:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9211: Fix adding new table to stale db due to concurrent reset
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14820 Change subject: IMPALA-9211: Fix adding new table to stale db due to concurrent reset .. IMPALA-9211: Fix adding new table to stale db due to concurrent reset When adding a new table to the catalog, we first get the db object from dbCache and then add the table into it. When doing reset(), i.e. global INVALIDATE METADATA, we replace the whole dbCache with a new one and load db and table names from HMS. These two operations have race conflicts so should both be protected by a exclusive lock, i.e. write lock of version lock. Currently, CatalogServiceCatalog.addTable() does not get the db object within the write lock, which may get a stale db object and add new table into it. This patch moves the operations into the protection of write lock. Tests: - Ran test_concurrent_ddls.py without errors for CreateTable like IMPALA-9135 found. Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/20/14820/1 -- To view, visit http://gerrit.cloudera.org:8080/14820 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I83d2e5f00eabe61a42c948ec1685ce29cdea1592 Gerrit-Change-Number: 14820 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang
