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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-HasComments: Yes
