[Impala-ASF-CR] [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

2017-12-04 Thread Dimitris Tsirogiannis (Code Review) 
Dimitris Tsirogiannis has abandoned this change. ( 
http://gerrit.cloudera.org:8080/8545 )

Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations
..


Abandoned

Resubmitted merged with the changes for impala-5538.
--
To view, visit http://gerrit.cloudera.org:8080/8545
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3
Gerrit-Change-Number: 8545
Gerrit-PatchSet: 3
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis

[Impala-ASF-CR] [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

2017-12-01 Thread Dimitris Tsirogiannis (Code Review) 
Hello Bharath Vissapragada, Alex Behm,

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

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

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

Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations
..

[PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

Problem: A long running table metadata operation (e.g. refresh) could
prevent any other metadata operation from making progress if it
coincided with the gather catalog updates operation. The problem was due
to the conservative locking scheme used when catalog updates were
collected. In particular, in order to collect a consistent snapshot of
metadata changes, the global catalog lock was held for the entire
duration of that operation.

Solution: To improve the concurrency of catalog operations the following
changes are performed:
* A range of catalog versions determines the catalog changes to be
  included in a catalog update. Any catalog changes that do not fall in
  the specified range are ignored (to be processed in subsequent catalog
  updates).
* The catalog allows metadata operations to make progress while collecting
  catalog updates.
* To prevent starvation of catalog updates (i.e. frequently updated
  catalog objects skipping catalog updates indefinitely), we keep track
  of the number of times a catalog object has skipped an update and if
  that number exceeds a threshold it is included in the next catalog
  update even if its version is not in the specified catalog update
  version range. Hence, the same catalog object may be sent in two
  consecutive catalog updates.

Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3
---
M be/src/exec/catalog-op-executor.cc
M be/src/exec/catalog-op-executor.h
M be/src/service/client-request-state.cc
M be/src/service/frontend.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M common/thrift/CatalogService.thrift
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogDeltaLog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectCache.java
A fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
A fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionQueue.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/DataSource.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/Function.java
M fe/src/main/java/org/apache/impala/catalog/HdfsCachePool.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Role.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
29 files changed, 1,417 insertions(+), 587 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3
Gerrit-Change-Number: 8545
Gerrit-PatchSet: 3
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis

[Impala-ASF-CR] [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

2017-12-01 Thread Dimitris Tsirogiannis (Code Review) 
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8545 )

Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations
..


Patch Set 2:

(21 comments)

http://gerrit.cloudera.org:8080/#/c/8545/2/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/8545/2/be/src/service/client-request-state.cc@187
PS2, Line 187:   reset_req.reset_metadata_params.__set_sync_ddl(
> Why do we need to set sync_ddl twice?
It's because we're not always consistent in how we process different requests. 
TCatalogOpRequest::sync_ddl is now a required field. At the same time some 
operations process the *params field and don't have access to the parent 
TCatalogOpRequest, so the value of the sync_ddl flag is lost.


http://gerrit.cloudera.org:8080/#/c/8545/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/8545/2/be/src/service/impala-server.cc@1430
PS2, Line 1430: LOG(INFO) << "Update applied with version: " << 
new_catalog_version
> Catalog update applied...
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionManager.java
File 
fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionManager.java:

http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionManager.java@29
PS2, Line 29: public class CatalogObjectVersionManager {
> Name is not very telling. Some alternatives:
Went with option #1.


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionManager.java@37
PS2, Line 37:   public void updateCatalogObjectVersions(long oldVersion, long 
newVersion) {
> Do you think it's worth checking the return codes of the calls to objectVer
Yes, added check on the remove path. The version should correspond to an 
existing object that is about to be removed, so the version should exist in the 
queue.


http://gerrit.cloudera.org:8080/#/c/8545/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/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@86
PS2, Line 86:  * and processing of DDL requests. For each DDL request, the 
CatalogServiceCatalog
> This description of the versioning scheme is not accurate anymore.
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@96
PS2, Line 96:  * - Delete log. The delete log records catalog objects that have 
been removed from the
> Let's give an intuition for the purpose of this log, e.g.:
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@97
PS2, Line 97:  *   catalog. An entry is added to this log every time an object 
is removed (e.g.
> An entry with a new version is added ...
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@99
PS2, Line 99:  *   be perfomed atomically. An entry is removed from this log 
from the topic update
> removed from this log by the topic update thread
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@102
PS2, Line 102:  *   been processed by the topic update thread. The topic update 
thread is the only one
> that have been included in a topic update.
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@105
PS2, Line 105:  *   Information per catalog object includes the number of times 
a catalog object has
> Each entry includes the number of times a catalog object has been omitted i
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@111
PS2, Line 111:  *   version that the issueing impalad must wait for in order to 
ensure that the effects
> typo: issuing
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@112
PS2, Line 112:  *   of this operation have been broadcast to all the 
coordinators. The time-based cleanup
> Why did we chose a time-based cleanup then? I think the time-based cleanup
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@129
PS2, Line 129:  * that should be employed if both the catalog lock and table 
locks need to be held at
> catalog lock -> version lock?
Done


http://gerrit.cloudera.org:8080/#/c/8545/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@183
PS2, Line 183:   // Version of the last topic update

[Impala-ASF-CR] [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

2017-11-27 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8545 )

Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations
..


Patch Set 1:

(1 comment)

Doing another full round now

http://gerrit.cloudera.org:8080/#/c/8545/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/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@606
PS1, Line 606: new 
CatalogUpdateLogEntry(updateEntry.getNumSkippedCatalogUpdates() + 1,
> Actually, the reason I am doing it like that is to avoid the need for synch
Synchronize between what? There's only one thread calling getCatalogDelta() and 
the background GC thread does not modify the TopicUpdateLogEntries.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3
Gerrit-Change-Number: 8545
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Comment-Date: Mon, 27 Nov 2017 22:29:00 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

2017-11-27 Thread Dimitris Tsirogiannis (Code Review) 
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8545 )

Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations
..


Patch Set 1:

(37 comments)

Addressed comments and fixed number of races. Run core and exhaustive tests.

http://gerrit.cloudera.org:8080/#/c/8545/1/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8545/1/be/src/service/impala-server.h@336
PS1, Line 336:   /// Wait until the last applied catalog update has been 
broadcasted to the
> broadcast
Done


http://gerrit.cloudera.org:8080/#/c/8545/1/be/src/service/impala-server.h@337
PS1, Line 337:   /// entire cluster or until the catalog service id has changed.
> entire cluster -> all coordinators
Done


http://gerrit.cloudera.org:8080/#/c/8545/1/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/8545/1/be/src/service/impala-server.cc@1462
PS1, Line 1462:   // TODO: What about query cancellation?
> Unfortunately, most DDL operations are not cancellable due to IMPALA-2568 s
Yeah, that was an old TODO. I'll remove it.


http://gerrit.cloudera.org:8080/#/c/8545/1/be/src/service/impala-server.cc@1503
PS1, Line 1503:   const TUniqueId& catalog_service_id = 
catalog_update_result.catalog_service_id;
> Document the waiting process for SYNC_DDL dance in the function comment of
Done


http://gerrit.cloudera.org:8080/#/c/8545/1/be/src/service/impala-server.cc@1507
PS1, Line 1507: // 'catalog_update_result' to determined when the effects 
of this operation
> to determine
Done


http://gerrit.cloudera.org:8080/#/c/8545/1/be/src/service/impala-server.cc@1512
PS1, Line 1512:   WaitForCatalogUpdate(catalog_update_result.version, 
catalog_service_id);
> What operations go down this path?
drop db/table if exists. Let me know if a comment is needed here.


http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
File fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java:

http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java@89
PS1, Line 89:   
CatalogObjectVersionManager.INSTANCE.removeCatalogObjectVersion(
> What's this new code for?
It is for maintaining all the catalog object versions of the local catalog 
cache up to date as the catalog is updated. This is for the sole purpose of 
being able to answer when the minimum version in the local cache is greater 
than some specified catalog update version (see INVALIDATE METADATA).


http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
File fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java:

http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java@20
PS1, Line 20: import java.util.concurrent.atomic.AtomicInteger;
> unused
Done


http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionManager.java
File 
fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionManager.java:

http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionManager.java@25
PS1, Line 25: public class CatalogObjectVersionManager {
> What's the purpose of this class? Do changes need to be atomic with respect
Added class comment. Changes are atomic with respect to the global version but 
this class is not responsible for enforcing that. All the calls to the methods 
of this class originate from the synchronized ImpaladCatalog::updateCatalog() 
call that is also responsible for updating the global version.


http://gerrit.cloudera.org:8080/#/c/8545/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/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@129
PS1, Line 129:   private final ReentrantReadWriteLock catalogLock_ = new 
ReentrantReadWriteLock(true);
> How do you feel about renaming this to versionLock_ to make it clear that t
Done


http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@159
PS1, Line 159:   // Version of the last sent catalog update. The version of an 
update is the
> Maybe I'm being dense, but I don't understand what this means. Can you clar
Rephrased it. Let me know if it's clear now.


http://gerrit.cloudera.org:8080/#/c/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@170
PS1, Line 170:   private int numCatalogUpdatesToGc_ = 
CATALOG_UPDATE_LOG_GC_FREQUENCY;
> What's the unit?
This is quite tricky. Let's talk offline.

[Impala-ASF-CR] [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

2017-11-27 Thread Dimitris Tsirogiannis (Code Review) 
Hello Bharath Vissapragada, Alex Behm,

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

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

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

Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations
..

[PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

Problem: A long running table metadata operation (e.g. refresh) could
prevent any other metadata operation from making progress if it
coincided with the gather catalog updates operation. The problem was due
to the conservative locking scheme used when catalog updates were
collected. In particular, in order to collect a consistent snapshot of
metadata changes, the global catalog lock was held for the entire
duration of that operation.

Solution: To improve the concurrency of catalog operations the following
changes are performed:
* A range of catalog versions determines the catalog changes to be
  included in a catalog update. Any catalog changes that do not fall in
  the specified range are ignored (to be processed in subsequent catalog
  updates).
* The catalog allows metadata operations to make progress while collecting
  catalog updates.
* To prevent starvation of catalog updates (i.e. frequently updated
  catalog objects skipping catalog updates indefinitely), we keep track
  of the number of times a catalog object has skipped an update and if
  that number exceeds a threshold it is included in the next catalog
  update even if its version is not in the specified catalog update
  version range. Hence, the same catalog object may be sent in two
  consecutive catalog updates.

Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3
---
M be/src/exec/catalog-op-executor.cc
M be/src/exec/catalog-op-executor.h
M be/src/service/client-request-state.cc
M be/src/service/frontend.cc
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M common/thrift/CatalogService.thrift
M common/thrift/Frontend.thrift
M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogDeltaLog.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObject.java
M fe/src/main/java/org/apache/impala/catalog/CatalogObjectCache.java
A fe/src/main/java/org/apache/impala/catalog/CatalogObjectImpl.java
A fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionManager.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/catalog/DataSource.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/Function.java
M fe/src/main/java/org/apache/impala/catalog/HdfsCachePool.java
M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
M fe/src/main/java/org/apache/impala/catalog/Role.java
M fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
29 files changed, 1,321 insertions(+), 534 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3
Gerrit-Change-Number: 8545
Gerrit-PatchSet: 2
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Dimitris Tsirogiannis

[Impala-ASF-CR] [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

2017-11-15 Thread Bharath Vissapragada (Code Review) 
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8545 )

Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8545/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/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@596
PS1, Line 596:   addTableToCatalogDeltaHelper(tbl, fromVersion, toVersion, 
resp);
> I think your assessment is correct.
I see, thanks for the explanation. I thought we are trying to make 
getCatalogDelta() non-conflicting too, since this contention can delay other 
Catalog updates like table loads etc. I understand the scope of this change 
better now, I have other comments that I'll flush out separately.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3
Gerrit-Change-Number: 8545
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Comment-Date: Thu, 16 Nov 2017 07:00:54 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations

2017-11-15 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8545 )

Change subject: [PREVIEW] IMPALA-5058: Improve concurrency of DDL/DML operations
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8545/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/8545/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@596
PS1, Line 596:   addTableToCatalogDeltaHelper(tbl, fromVersion, toVersion, 
resp);
> Maybe I'm misunderstanding something but I still see the lock contention on
I think your assessment is correct.

We are not trying to make getCatalogDelta() completely non-blocking. As you've 
correctly pointed out it can still block on a table lock.

The problem we are trying to solve is that non-conflicting operations should 
not conflict. Before this change the global lock was held for the entire 
duration of getCatalogDelta(). If a long-running operation is running against 
tbl A, and the thread executing getCatalogDelta() needs to wait to get the 
table lock on A, then no concurrent DDL whatsoever is possible due to the 
global lock. That's the main problem: a long running operation on A can prevent 
concurrent DDL against anything else.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3032437f83d39bcc8cff14d897c7c106a4ab62d3
Gerrit-Change-Number: 8545
Gerrit-PatchSet: 1
Gerrit-Owner: Dimitris Tsirogiannis 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Comment-Date: Thu, 16 Nov 2017 06:42:10 +
Gerrit-HasComments: Yes