From a479e9d08c2afa1d15341a8ec00823f8811c60c7 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Wed, 12 Jun 2019 14:01:26 -0700 Subject: [PATCH 1/3] Fix concurrent access to lazy PartitionSpec methods. --- .../org/apache/iceberg/PartitionSpec.java | 49 +++++++++++++------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java index 3d7475b4a2cb..6b13a7521edb 100644 --- a/api/src/main/java/org/apache/iceberg/PartitionSpec.java +++ b/api/src/main/java/org/apache/iceberg/PartitionSpec.java @@ -118,12 +118,18 @@ public Types.StructType partitionType() { public Class[] javaClasses() { if (lazyJavaClasses == null) { - this.lazyJavaClasses = new Class[fields.length]; - for (int i = 0; i < fields.length; i += 1) { - PartitionField field = fields[i]; - Type sourceType = schema.findType(field.sourceId()); - Type result = field.transform().getResultType(sourceType); - lazyJavaClasses[i] = result.typeId().javaClass(); + synchronized (this) { + if (lazyJavaClasses == null) { + Class[] classes = new Class[fields.length]; + for (int i = 0; i < fields.length; i += 1) { + PartitionField field = fields[i]; + Type sourceType = schema.findType(field.sourceId()); + Type result = field.transform().getResultType(sourceType); + classes[i] = result.typeId().javaClass(); + } + + this.lazyJavaClasses = classes; + } } } @@ -209,18 +215,26 @@ public int hashCode() { private List lazyFieldList() { if (fieldList == null) { - this.fieldList = ImmutableList.copyOf(fields); + synchronized (this) { + if (fieldList == null) { + this.fieldList = ImmutableList.copyOf(fields); + } + } } return fieldList; } private Map lazyFieldsByName() { if (fieldsByName == null) { - ImmutableMap.Builder builder = ImmutableMap.builder(); - for (PartitionField field : fields) { - builder.put(field.name(), field); + synchronized (this) { + if (fieldsByName == null) { + ImmutableMap.Builder builder = ImmutableMap.builder(); + for (PartitionField field : fields) { + builder.put(field.name(), field); + } + this.fieldsByName = builder.build(); + } } - this.fieldsByName = builder.build(); } return fieldsByName; @@ -228,10 +242,15 @@ private Map lazyFieldsByName() { private ListMultimap lazyFieldsBySourceId() { if (fieldsBySourceId == null) { - this.fieldsBySourceId = Multimaps - .newListMultimap(Maps.newHashMap(), () -> Lists.newArrayListWithCapacity(fields.length)); - for (PartitionField field : fields) { - fieldsBySourceId.put(field.sourceId(), field); + synchronized (this) { + if (fieldsBySourceId == null) { + ListMultimap multiMap = Multimaps + .newListMultimap(Maps.newHashMap(), () -> Lists.newArrayListWithCapacity(fields.length)); + for (PartitionField field : fields) { + multiMap.put(field.sourceId(), field); + } + this.fieldsBySourceId = multiMap; + } } } From 92710c9c330b3380926aefd8d0f49e2903cdae4b Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Fri, 14 Jun 2019 17:16:51 -0700 Subject: [PATCH 2/3] Add volatile. --- api/src/main/java/org/apache/iceberg/PartitionSpec.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java index 6b13a7521edb..169985f57a63 100644 --- a/api/src/main/java/org/apache/iceberg/PartitionSpec.java +++ b/api/src/main/java/org/apache/iceberg/PartitionSpec.java @@ -55,10 +55,10 @@ public class PartitionSpec implements Serializable { // this is ordered so that DataFile has a consistent schema private final int specId; private final PartitionField[] fields; - private transient ListMultimap fieldsBySourceId = null; - private transient Map fieldsByName = null; - private transient Class[] lazyJavaClasses = null; - private transient List fieldList = null; + private volatile transient ListMultimap fieldsBySourceId = null; + private volatile transient Map fieldsByName = null; + private volatile transient Class[] lazyJavaClasses = null; + private volatile transient List fieldList = null; private PartitionSpec(Schema schema, int specId, List fields) { this.schema = schema; From f73960dc431b459d1b7ef03f01b64ebb9f3fb9ed Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Mon, 17 Jun 2019 10:01:29 -0700 Subject: [PATCH 3/3] Fix modifier order. --- api/src/main/java/org/apache/iceberg/PartitionSpec.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/iceberg/PartitionSpec.java b/api/src/main/java/org/apache/iceberg/PartitionSpec.java index 169985f57a63..5013119f61cb 100644 --- a/api/src/main/java/org/apache/iceberg/PartitionSpec.java +++ b/api/src/main/java/org/apache/iceberg/PartitionSpec.java @@ -55,10 +55,10 @@ public class PartitionSpec implements Serializable { // this is ordered so that DataFile has a consistent schema private final int specId; private final PartitionField[] fields; - private volatile transient ListMultimap fieldsBySourceId = null; - private volatile transient Map fieldsByName = null; - private volatile transient Class[] lazyJavaClasses = null; - private volatile transient List fieldList = null; + private transient volatile ListMultimap fieldsBySourceId = null; + private transient volatile Map fieldsByName = null; + private transient volatile Class[] lazyJavaClasses = null; + private transient volatile List fieldList = null; private PartitionSpec(Schema schema, int specId, List fields) { this.schema = schema;