[Impala-ASF-CR] WIP: IMPALA-7434. Handle invalidation races in CatalogdMetaProvider

2019-06-18 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13664 )

Change subject: WIP: IMPALA-7434. Handle invalidation races in 
CatalogdMetaProvider
..


Patch Set 1:

> Patch Set 1:
>
> I'm trying to understand the issue here. Apart from the versioning problem, 
> is the thread-safety issue in loadWithCaching() described in 
> https://softwaremill.com/race-condition-cache-guava-caffeine/ not a problem 
> here?

That's exactly the problem this is fixing. It ensures that an invalidate 
concurrent with loadWithCaching() will cause the concurrent load to _not_ write 
its result to the cache.

In terms of multiple threads calling loadWithCaching() at the same time, I 
think our code appropriately ensures that the first thread who gets there 
inserts a Future, and the second thread piggy-backs onto that same future.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70f377db88e204825a909389f28dc3451815235c
Gerrit-Change-Number: 13664
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 18 Jun 2019 17:04:28 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: IMPALA-7434. Handle invalidation races in CatalogdMetaProvider

2019-06-18 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13664 )

Change subject: WIP: IMPALA-7434. Handle invalidation races in 
CatalogdMetaProvider
..


Patch Set 1:

I'm trying to understand the issue here. Apart from the versioning problem, is 
the thread-safety issue in loadWithCaching() described in 
https://softwaremill.com/race-condition-cache-guava-caffeine/ not a problem 
here?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70f377db88e204825a909389f28dc3451815235c
Gerrit-Change-Number: 13664
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 18 Jun 2019 17:01:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: IMPALA-7434. Handle invalidation races in CatalogdMetaProvider

2019-06-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13664 )

Change subject: WIP: IMPALA-7434. Handle invalidation races in 
CatalogdMetaProvider
..


Patch Set 1:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70f377db88e204825a909389f28dc3451815235c
Gerrit-Change-Number: 13664
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Vihang Karajgaonkar 
Gerrit-Comment-Date: Tue, 18 Jun 2019 00:48:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] WIP: IMPALA-7434. Handle invalidation races in CatalogdMetaProvider

2019-06-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13664 )

Change subject: WIP: IMPALA-7434. Handle invalidation races in 
CatalogdMetaProvider
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13664/1/tests/custom_cluster/test_local_catalog.py
File tests/custom_cluster/test_local_catalog.py:

http://gerrit.cloudera.org:8080/#/c/13664/1/tests/custom_cluster/test_local_catalog.py@329
PS1, Line 329: c
flake8: E306 expected 1 blank line before a nested definition, found 0


http://gerrit.cloudera.org:8080/#/c/13664/1/tests/custom_cluster/test_local_catalog.py@334
PS1, Line 334: d
flake8: E306 expected 1 blank line before a nested definition, found 0



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70f377db88e204825a909389f28dc3451815235c
Gerrit-Change-Number: 13664
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 18 Jun 2019 00:09:07 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: IMPALA-7434. Handle invalidation races in CatalogdMetaProvider

2019-06-17 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13664 )

Change subject: WIP: IMPALA-7434. Handle invalidation races in 
CatalogdMetaProvider
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13664/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13664/1//COMMIT_MSG@7
PS1, Line 7: WIP: IMPALA-7434. Handle invalidation races in CatalogdMetaProvider
oops this should say IMPALA-7534.


http://gerrit.cloudera.org:8080/#/c/13664/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java:

http://gerrit.cloudera.org:8080/#/c/13664/1/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@240
PS1, Line 240: 7543
also got it wrong here :)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I70f377db88e204825a909389f28dc3451815235c
Gerrit-Change-Number: 13664
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Tue, 18 Jun 2019 00:09:26 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] WIP: IMPALA-7434. Handle invalidation races in CatalogdMetaProvider

2019-06-17 Thread Todd Lipcon (Code Review)
Hello Bharath Vissapragada,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: WIP: IMPALA-7434. Handle invalidation races in 
CatalogdMetaProvider
..

WIP: IMPALA-7434. Handle invalidation races in CatalogdMetaProvider

This handles a race condition in which a cache invalidation concurrent
with a cache load would potentially be skipped, causing out-of-date data
to persist in the cache. This would present itself as spurious "table
not found" errors.

A new test case triggers the issue reliably by injecting latency into
the metadata fetch RPC and running DDLs concurrently on the same
database across 8 threads. With the fix, the test passes reliably.

WIP because it probably needs a little more cleanup and testing, but
posting for early review

Change-Id: I70f377db88e204825a909389f28dc3451815235c
---
M be/src/exec/catalog-op-executor.cc
M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
M tests/custom_cluster/test_local_catalog.py
3 files changed, 113 insertions(+), 23 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/13664/1
--
To view, visit http://gerrit.cloudera.org:8080/13664
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I70f377db88e204825a909389f28dc3451815235c
Gerrit-Change-Number: 13664
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Bharath Vissapragada