impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4902: Copy parameters map in HdfsPartition.toThrift().
Date Fri, 24 Feb 2017 05:54:50 GMT
Alex Behm has posted comments on this change.

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


Patch Set 2:

(2 comments)

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

PS2, Line 765: One thread
             :     // may try to serialize the returned THdfsPartition after releasing the
table's lock,
             :     // and another thread doing DDL may modify the map.
> Are we sure this is needed? When do we call HdfsPartition.toThrift() outsid
The problem is that the returned THdfsPartition gets serialized at some point, while not holding
the table lock. That serialization code walks the parameters map and if another operation
modifies the map at that point, then you get a ConcurrentModificationException.

I don't think it's practical to hold the table lock until the returned thrift object is serialized.


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

PS2, Line 26: import java.util.concurrent.locks.ReentrantReadWriteLock;
> I don't think you need that.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic11277ad5512d2431cd3cc791715917c95395ddf
Gerrit-PatchSet: 2
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-HasComments: Yes

Mime
View raw message