This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push: new 82f315e52d Fix race condition in shared literal transform functions (#13916) 82f315e52d is described below commit 82f315e52d01f8fdd84fd9a8cf8f3ae4884ab9f7 Author: Xiaotian (Jackie) Jiang <17555551+jackie-ji...@users.noreply.github.com> AuthorDate: Thu Aug 29 21:11:22 2024 -0700 Fix race condition in shared literal transform functions (#13916) --- .../function/ArrayLiteralTransformFunction.java | 15 +++++++++------ .../function/GenerateArrayTransformFunction.java | 13 +++++++++---- .../transform/function/LiteralTransformFunction.java | 19 +++++++++++-------- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ArrayLiteralTransformFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ArrayLiteralTransformFunction.java index 084d34bf2e..d14f26f160 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ArrayLiteralTransformFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ArrayLiteralTransformFunction.java @@ -48,12 +48,15 @@ public class ArrayLiteralTransformFunction implements TransformFunction { private final double[] _doubleArrayLiteral; private final String[] _stringArrayLiteral; - // literals may be shared but values are intentionally not volatile as assignment races are benign - private int[][] _intArrayResult; - private long[][] _longArrayResult; - private float[][] _floatArrayResult; - private double[][] _doubleArrayResult; - private String[][] _stringArrayResult; + // NOTE: + // This class can be shared across multiple threads, and the result arrays are lazily initialized and cached. They + // need to be declared as volatile to ensure instructions are not reordered, or some threads might see uninitialized + // arrays. + private volatile int[][] _intArrayResult; + private volatile long[][] _longArrayResult; + private volatile float[][] _floatArrayResult; + private volatile double[][] _doubleArrayResult; + private volatile String[][] _stringArrayResult; public ArrayLiteralTransformFunction(LiteralContext literalContext) { _dataType = literalContext.getType(); diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/GenerateArrayTransformFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/GenerateArrayTransformFunction.java index 44632867ac..0963917495 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/GenerateArrayTransformFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/GenerateArrayTransformFunction.java @@ -41,10 +41,15 @@ public class GenerateArrayTransformFunction implements TransformFunction { private final long[] _longArrayLiteral; private final float[] _floatArrayLiteral; private final double[] _doubleArrayLiteral; - private int[][] _intArrayResult; - private long[][] _longArrayResult; - private float[][] _floatArrayResult; - private double[][] _doubleArrayResult; + + // NOTE: + // This class can be shared across multiple threads, and the result arrays are lazily initialized and cached. They + // need to be declared as volatile to ensure instructions are not reordered, or some threads might see uninitialized + // arrays. + private volatile int[][] _intArrayResult; + private volatile long[][] _longArrayResult; + private volatile float[][] _floatArrayResult; + private volatile double[][] _doubleArrayResult; public GenerateArrayTransformFunction(List<ExpressionContext> literalContexts) { Preconditions.checkNotNull(literalContexts); diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LiteralTransformFunction.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LiteralTransformFunction.java index 80db464546..59aa089056 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LiteralTransformFunction.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/LiteralTransformFunction.java @@ -40,14 +40,17 @@ public class LiteralTransformFunction implements TransformFunction { private final LiteralContext _literalContext; - // literals may be shared but values are intentionally not volatile as assignment races are benign - private int[] _intResult; - private long[] _longResult; - private float[] _floatResult; - private double[] _doubleResult; - private BigDecimal[] _bigDecimalResult; - private String[] _stringResult; - private byte[][] _bytesResult; + // NOTE: + // This class can be shared across multiple threads, and the result arrays are lazily initialized and cached. They + // need to be declared as volatile to ensure instructions are not reordered, or some threads might see uninitialized + // arrays. + private volatile int[] _intResult; + private volatile long[] _longResult; + private volatile float[] _floatResult; + private volatile double[] _doubleResult; + private volatile BigDecimal[] _bigDecimalResult; + private volatile String[] _stringResult; + private volatile byte[][] _bytesResult; public LiteralTransformFunction(LiteralContext literalContext) { _literalContext = literalContext; --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org