Skipped locking for NoopCompressor

linkedin · Nov 15, 2024 · 8e8ec26 · 8e8ec26
1 parent d8064d4
commit 8e8ec26
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 7 deletions.
diff --git a/...al/venice-client-common/src/main/java/com/linkedin/venice/compression/NoopCompressor.java b/...al/venice-client-common/src/main/java/com/linkedin/venice/compression/NoopCompressor.java
@@ -6,41 +6,64 @@
 import java.nio.ByteBuffer;
 
 
+/**
+ * Locking is not necessary for {@link NoopCompressor}, so this class overrides all the public APIs to avoid locking.
+ */
 public class NoopCompressor extends VeniceCompressor {
   public NoopCompressor() {
     super(CompressionStrategy.NO_OP);
   }
 
   @Override
-  protected byte[] compressInternal(byte[] data) throws IOException {
+  public byte[] compress(byte[] data) throws IOException {
     return data;
   }
 
   @Override
-  protected ByteBuffer compressInternal(ByteBuffer data, int startPositionOfOutput) throws IOException {
+  protected byte[] compressInternal(byte[] data) throws IOException {
+    throw new UnsupportedOperationException("compressInternal");
+  }
+
+  @Override
+  public ByteBuffer compress(ByteBuffer data, int startPositionOfOutput) throws IOException {
     if (startPositionOfOutput != 0) {
       throw new UnsupportedOperationException("Compression with front padding is not supported for NO_OP.");
     }
     return data;
   }
 
+  @Override
+  protected ByteBuffer compressInternal(ByteBuffer src, int startPositionOfOutput) throws IOException {
+    throw new UnsupportedOperationException("compressInternal");
+  }
+
   @Override
   public int hashCode() {
     return super.hashCode();
   }
 
   @Override
-  protected ByteBuffer decompressInternal(ByteBuffer data) throws IOException {
+  public ByteBuffer decompress(ByteBuffer data) throws IOException {
     return data;
   }
 
   @Override
-  protected ByteBuffer decompressInternal(byte[] data, int offset, int length) throws IOException {
+  protected ByteBuffer decompressInternal(ByteBuffer data) throws IOException {
+    throw new UnsupportedOperationException("decompressInternal");
+  }
+
+  @Override
+  public ByteBuffer decompress(byte[] data, int offset, int length) throws IOException {
     return ByteBuffer.wrap(data, offset, length);
   }
 
   @Override
-  protected ByteBuffer decompressAndPrependSchemaHeaderInternal(byte[] data, int offset, int length, int schemaHeader)
+  protected ByteBuffer decompressInternal(byte[] data, int offset, int length) throws IOException {
+    throw new UnsupportedOperationException("decompressInternal");
+  }
+
+  @Override
+  public ByteBuffer decompressAndPrependSchemaHeader(byte[] data, int offset, int length, int schemaHeader)
       throws IOException {
     if (offset < SCHEMA_HEADER_LENGTH) {
       throw new VeniceException("Start offset does not have enough room for schema header.");
@@ -51,15 +74,31 @@ protected ByteBuffer decompressAndPrependSchemaHeaderInternal(byte[] data, int o
   }
 
   @Override
-  protected InputStream decompressInternal(InputStream inputStream) throws IOException {
+  protected ByteBuffer decompressAndPrependSchemaHeaderInternal(byte[] data, int offset, int length, int schemaHeader)
+      throws IOException {
+    throw new UnsupportedOperationException("decompressAndPrependSchemaHeaderInternal");
+  }
+
+  @Override
+  public InputStream decompress(InputStream inputStream) throws IOException {
     return inputStream;
   }
 
   @Override
-  protected void closeInternal() throws IOException {
+  protected InputStream decompressInternal(InputStream inputStream) throws IOException {
+    throw new UnsupportedOperationException("decompressInternal");
+  }
+
+  @Override
+  public void close() throws IOException {
     // do nothing
   }
 
+  @Override
+  protected void closeInternal() throws IOException {
+    throw new UnsupportedOperationException("closeInternal");
+  }
+
   @Override
   public boolean equals(Object o) {
     if (o == this) {

diff --git a/.../venice-client-common/src/main/java/com/linkedin/venice/compression/VeniceCompressor.java b/.../venice-client-common/src/main/java/com/linkedin/venice/compression/VeniceCompressor.java
@@ -13,6 +13,9 @@ public abstract class VeniceCompressor implements Closeable {
   protected static final int SCHEMA_HEADER_LENGTH = ByteUtils.SIZE_OF_INT;
   private final CompressionStrategy compressionStrategy;
   private boolean isClosed = false;
+  /**
+   * To avoid the race condition between 'compress'/'decompress' operation and 'close'.
+   */
   private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock();
 
   protected VeniceCompressor(CompressionStrategy compressionStrategy) {