Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix ConcurrentModificationException in JDK11 (#8391) 

When building column/dimension selectors, calling computeIfAbsent can
cause the applied function to modify the same cache through virtual
column references. The JDK11 map implementation detects this change and
will throw an exception.

This fix – while not as elegant – breaks the single call into two
steps to avoid this problem.
This commit is contained in:
Xavier Léauté 2019-08-24 18:24:50 -04:00 committed by GitHub
parent 5c7803fe6b
commit 20f7db5d22
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 59 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java
View File

@ -32,6 +32,7 @@ import org.apache.druid.segment.data.ReadableOffset;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.function.Function;
/** /**
* The basic implementation of {@link ColumnSelectorFactory} over a historical segment (i. e. {@link QueryableIndex}). * The basic implementation of {@link ColumnSelectorFactory} over a historical segment (i. e. {@link QueryableIndex}).
@ -76,9 +77,7 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory
@Override @Override
public DimensionSelector makeDimensionSelector(DimensionSpec dimensionSpec) public DimensionSelector makeDimensionSelector(DimensionSpec dimensionSpec)
{ {
return dimensionSelectorCache.computeIfAbsent( Function<DimensionSpec, DimensionSelector> mappingFunction = spec -> {
dimensionSpec,
spec -> {
if (virtualColumns.exists(spec.getDimension())) { if (virtualColumns.exists(spec.getDimension())) {
DimensionSelector dimensionSelector = virtualColumns.makeDimensionSelector(dimensionSpec, index, offset); DimensionSelector dimensionSelector = virtualColumns.makeDimensionSelector(dimensionSpec, index, offset);
if (dimensionSelector == null) { if (dimensionSelector == null) {
@ -89,8 +88,18 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory
} }
return spec.decorate(makeDimensionSelectorUndecorated(spec)); return spec.decorate(makeDimensionSelectorUndecorated(spec));
};
// We cannot use dimensionSelectorCache.computeIfAbsent() here since the function being
// applied may modify the dimensionSelectorCache itself through virtual column references,
// triggering a ConcurrentModificationException in JDK 9 and above.
DimensionSelector dimensionSelector = dimensionSelectorCache.get(dimensionSpec);
if (dimensionSelector == null) {
dimensionSelector = mappingFunction.apply(dimensionSpec);
dimensionSelectorCache.put(dimensionSpec, dimensionSelector);
} }
);
return dimensionSelector;
} }
private DimensionSelector makeDimensionSelectorUndecorated(DimensionSpec dimensionSpec) private DimensionSelector makeDimensionSelectorUndecorated(DimensionSpec dimensionSpec)
@ -124,9 +133,7 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory
@Override @Override
public ColumnValueSelector<?> makeColumnValueSelector(String columnName) public ColumnValueSelector<?> makeColumnValueSelector(String columnName)
{ {
return valueSelectorCache.computeIfAbsent( Function<String, ColumnValueSelector<?>> mappingFunction = name -> {
columnName,
name -> {
if (virtualColumns.exists(columnName)) { if (virtualColumns.exists(columnName)) {
ColumnValueSelector<?> selector = virtualColumns.makeColumnValueSelector(columnName, index, offset); ColumnValueSelector<?> selector = virtualColumns.makeColumnValueSelector(columnName, index, offset);
if (selector == null) { if (selector == null) {
@ -143,8 +150,18 @@ class QueryableIndexColumnSelectorFactory implements ColumnSelectorFactory
} else { } else {
return NilColumnValueSelector.instance(); return NilColumnValueSelector.instance();
} }
};
// We cannot use valueSelectorCache.computeIfAbsent() here since the function being
// applied may modify the valueSelectorCache itself through virtual column references,
// triggering a ConcurrentModificationException in JDK 9 and above.
ColumnValueSelector<?> columnValueSelector = valueSelectorCache.get(columnName);
if (columnValueSelector == null) {
columnValueSelector = mappingFunction.apply(columnName);
valueSelectorCache.put(columnName, columnValueSelector);
} }
);
return columnValueSelector;
} }
@Nullable @Nullable

10
processing/src/main/java/org/apache/druid/segment/incremental/OnheapIncrementalIndex.java
View File

@ -414,11 +414,17 @@ public class OnheapIncrementalIndex extends IncrementalIndex<Aggregator>
@Override @Override
public ColumnValueSelector<?> makeColumnValueSelector(String columnName) public ColumnValueSelector<?> makeColumnValueSelector(String columnName)
{ {
final ColumnValueSelector existing = columnSelectorMap.get(columnName); ColumnValueSelector existing = columnSelectorMap.get(columnName);
if (existing != null) { if (existing != null) {
return existing; return existing;
} }
return columnSelectorMap.computeIfAbsent(columnName, delegate::makeColumnValueSelector);
// We cannot use columnSelectorMap.computeIfAbsent(columnName, delegate::makeColumnValueSelector)
// here since makeColumnValueSelector may modify the columnSelectorMap itself through
// virtual column references, triggering a ConcurrentModificationException in JDK 9 and above.
ColumnValueSelector<?> columnValueSelector = delegate.makeColumnValueSelector(columnName);
existing = columnSelectorMap.putIfAbsent(columnName, columnValueSelector);
return existing != null ? existing : columnValueSelector;
} }
@Nullable @Nullable