You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org> on 2018/10/18 23:32:24 UTC

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11732


Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................

IMPALA-7717: Handle concurrent partition changes in local catalog mode

Current code throws a RuntimeException (RTE) when partial fetch RPCs
looking up partition metadata and the corresponding partition ID is
missing on the Catalog server. There are a couple of cases here.

1. The partition could be genuinely missing as it was dropped by a
   concurrent operation.
2. Partial fetch RPCs lookup partitions by IDs instead of names. This is
   problematic since the IDs can change over the lifetime of a table.

In both the cases, throwing a RTE is not the right approach and for (2)
we need to transparently retry the fetch with the new partition ID.

We eventually need to fix (2) as looking up by partition ID is not the
right approach.

Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Testing: Updated an e-e test which fails without the patch.
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/custom_cluster/test_local_catalog.py
4 files changed, 27 insertions(+), 53 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/11732/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 5:

Aborted this since it might clash with https://gerrit.cloudera.org/#/c/11608/ that just got submitted. Will rebase and kick off another build


-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 5
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 00:49:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

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/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................

IMPALA-7717: Handle concurrent partition changes in local catalog mode

Current code throws a RuntimeException (RTE) when partial fetch RPCs
looking up partition metadata and the corresponding partition ID is
missing on the Catalog server. There are a couple of cases here.

1. The partition could be genuinely missing as it was dropped by a
   concurrent operation.
2. Partial fetch RPCs lookup partitions by IDs instead of names. This is
   problematic since the IDs can change over the lifetime of a table.

In both the cases, throwing a RTE is not the right approach and for (2)
we need to transparently retry the fetch with the new partition ID.

We eventually need to fix (2) as looking up by partition ID is not the
right approach.

Testing: Updated an e-e test which fails without the patch.

Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Reviewed-on: http://gerrit.cloudera.org:8080/11732
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M tests/custom_cluster/test_local_catalog.py
5 files changed, 27 insertions(+), 58 deletions(-)

Approvals:
  Bharath Vissapragada: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 8
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1099/ : 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/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 2
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 00:06:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Adrian Ng, Impala Public Jenkins, Vuk Ercegovac, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11732

to look at the new patch set (#4).

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................

IMPALA-7717: Handle concurrent partition changes in local catalog mode

Current code throws a RuntimeException (RTE) when partial fetch RPCs
looking up partition metadata and the corresponding partition ID is
missing on the Catalog server. There are a couple of cases here.

1. The partition could be genuinely missing as it was dropped by a
   concurrent operation.
2. Partial fetch RPCs lookup partitions by IDs instead of names. This is
   problematic since the IDs can change over the lifetime of a table.

In both the cases, throwing a RTE is not the right approach and for (2)
we need to transparently retry the fetch with the new partition ID.

We eventually need to fix (2) as looking up by partition ID is not the
right approach.

Testing: Updated an e-e test which fails without the patch.

Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/custom_cluster/test_local_catalog.py
4 files changed, 24 insertions(+), 53 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/11732/4
-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Adrian Ng, Impala Public Jenkins, Vuk Ercegovac, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11732

to look at the new patch set (#6).

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................

IMPALA-7717: Handle concurrent partition changes in local catalog mode

Current code throws a RuntimeException (RTE) when partial fetch RPCs
looking up partition metadata and the corresponding partition ID is
missing on the Catalog server. There are a couple of cases here.

1. The partition could be genuinely missing as it was dropped by a
   concurrent operation.
2. Partial fetch RPCs lookup partitions by IDs instead of names. This is
   problematic since the IDs can change over the lifetime of a table.

In both the cases, throwing a RTE is not the right approach and for (2)
we need to transparently retry the fetch with the new partition ID.

We eventually need to fix (2) as looking up by partition ID is not the
right approach.

Testing: Updated an e-e test which fails without the patch.

Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/custom_cluster/test_local_catalog.py
4 files changed, 24 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/11732/6
-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1104/ : 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/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 05:54:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 7: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 09:22:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 7: Code-Review+2

Had to update a test. Carrying +2.


-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 05:23:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3333/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 01:26:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 6: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3333/


-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 05:21:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Adrian Ng, Vuk Ercegovac, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11732

to look at the new patch set (#3).

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................

IMPALA-7717: Handle concurrent partition changes in local catalog mode

Current code throws a RuntimeException (RTE) when partial fetch RPCs
looking up partition metadata and the corresponding partition ID is
missing on the Catalog server. There are a couple of cases here.

1. The partition could be genuinely missing as it was dropped by a
   concurrent operation.
2. Partial fetch RPCs lookup partitions by IDs instead of names. This is
   problematic since the IDs can change over the lifetime of a table.

In both the cases, throwing a RTE is not the right approach and for (2)
we need to transparently retry the fetch with the new partition ID.

We eventually need to fix (2) as looking up by partition ID is not the
right approach.

Testing: Updated an e-e test which fails without the patch.

Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/custom_cluster/test_local_catalog.py
4 files changed, 27 insertions(+), 53 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/11732/3
-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 3: Code-Review+2

(4 comments)

looks, several small comments.

http://gerrit.cloudera.org:8080/#/c/11732/3/common/thrift/CatalogService.thrift
File common/thrift/CatalogService.thrift:

http://gerrit.cloudera.org:8080/#/c/11732/3/common/thrift/CatalogService.thrift@384
PS3, Line 384: 1. The partition that the partial fetch RPC is looking for has already been dropped.
this is true of all the *_NOT_FOUND cases. bring this comment to the top of the enum?


http://gerrit.cloudera.org:8080/#/c/11732/3/common/thrift/CatalogService.thrift@386
PS3, Line 386: can potentially 
remove (they do change, easily, often)


http://gerrit.cloudera.org:8080/#/c/11732/3/common/thrift/CatalogService.thrift@387
PS3, Line 387: a 
remove


http://gerrit.cloudera.org:8080/#/c/11732/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/11732/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1716
PS3, Line 1716: new TG
good catch to not use "resp" since it may send back lots of stuff that will not be used.



-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 23:46:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1102/ : 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/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 02:00:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1101/ : 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/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 4
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 00:38:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11732/3/common/thrift/CatalogService.thrift
File common/thrift/CatalogService.thrift:

http://gerrit.cloudera.org:8080/#/c/11732/3/common/thrift/CatalogService.thrift@384
PS3, Line 384: 1. The partition that the partial fetch RPC is looking for has already been dropped.
> this is true of all the *_NOT_FOUND cases. bring this comment to the top of
It is probably kinda obvious from _NOT_FOUND part. Omitting it. Just retaining (2)


http://gerrit.cloudera.org:8080/#/c/11732/3/common/thrift/CatalogService.thrift@386
PS3, Line 386: can potentially 
> remove (they do change, easily, often)
Done


http://gerrit.cloudera.org:8080/#/c/11732/3/common/thrift/CatalogService.thrift@387
PS3, Line 387: a 
> remove
Done


http://gerrit.cloudera.org:8080/#/c/11732/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/11732/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1716
PS3, Line 1716: new TG
> good catch to not use "resp" since it may send back lots of stuff that will
Yep.



-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 3
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 00:06:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Hello Adrian Ng, Impala Public Jenkins, Vuk Ercegovac, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11732

to look at the new patch set (#7).

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................

IMPALA-7717: Handle concurrent partition changes in local catalog mode

Current code throws a RuntimeException (RTE) when partial fetch RPCs
looking up partition metadata and the corresponding partition ID is
missing on the Catalog server. There are a couple of cases here.

1. The partition could be genuinely missing as it was dropped by a
   concurrent operation.
2. Partial fetch RPCs lookup partitions by IDs instead of names. This is
   problematic since the IDs can change over the lifetime of a table.

In both the cases, throwing a RTE is not the right approach and for (2)
we need to transparently retry the fetch with the new partition ID.

We eventually need to fix (2) as looking up by partition ID is not the
right approach.

Testing: Updated an e-e test which fails without the patch.

Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
---
M common/thrift/CatalogService.thrift
M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoTest.java
M tests/custom_cluster/test_local_catalog.py
5 files changed, 27 insertions(+), 58 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/32/11732/7
-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3332/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 5
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 00:09:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7717: Handle concurrent partition changes in local catalog mode

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11732 )

Change subject: IMPALA-7717: Handle concurrent partition changes in local catalog mode
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3334/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11732
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2aa103ee159ce9478af9b5b27b36bc0cc286f442
Gerrit-Change-Number: 11732
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Adrian Ng (389)
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 05:23:28 +0000
Gerrit-HasComments: No