impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Impala Public Jenkins (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4902: Copy parameters map in HdfsPartition.toThrift().
Date Fri, 24 Feb 2017 10:18:39 GMT
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-4902: Copy parameters map in HdfsPartition.toThrift().
......................................................................


IMPALA-4902: Copy parameters map in HdfsPartition.toThrift().

The bug: When generating the toThrift() of an HdfsTable,
each THdfsPartition used to contain a reference to its
partition's parameters map. As a result, one thread trying
to serialize a thrift table returned by toThrift() could
conflict with another thread updating the parameters maps of
the table partitions. Here are a few examples of operations
that may modify the parameters map:
COMPUTE [INCREMENTAL] STATS, DROP STATS,
ALTER TABLE SET TBLPROPERTIES, ALTER TABLE SET CACHED, etc.

The fix: Create a shallow copy of the parameters map in
HdfsPartition.toThrift(). This means that toThrift() itself
must be protected from concurrent modifications to the
parameters map. Callers of toThrift() are now required
to hold the table lock. One place where the lock was not
already held needed to be adjusted.

Testing:
- I was unable to reproduce the issue locally, but the stacks
  from the JIRAs point directly to the parameters map, and
  the races are pretty obvious from looking at the code.
- Passed a core/hdfs private run.

Change-Id: Ic11277ad5512d2431cd3cc791715917c95395ddf
Reviewed-on: http://gerrit.cloudera.org:8080/6127
Reviewed-by: Alex Behm <alex.behm@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/HdfsPartition.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/test/java/org/apache/impala/catalog/CatalogObjectToFromThriftTest.java
5 files changed, 122 insertions(+), 101 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Alex Behm: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic11277ad5512d2431cd3cc791715917c95395ddf
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins

Mime
View raw message