Skip to content

HBASE-28390 WAL value compression fails for cells with large values #5696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bbeaudreault merged 1 commit into from
Feb 23, 2024
Merged

HBASE-28390 WAL value compression fails for cells with large values #5696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions ...rc/test/java/org/apache/hadoop/hbase/io/compress/aircompressor/TestWALCompressionLz4.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -29,10 +28,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionLz4 extends CompressedWALTestBase {
Expand All @@ -41,9 +37,6 @@ public class TestWALCompressionLz4 extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionLz4.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
Expand All @@ -60,10 +53,4 @@ public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
14 changes: 0 additions & 14 deletions ...rc/test/java/org/apache/hadoop/hbase/io/compress/aircompressor/TestWALCompressionLzo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -29,10 +28,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionLzo extends CompressedWALTestBase {
Expand All @@ -41,9 +37,6 @@ public class TestWALCompressionLzo extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionLzo.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
Expand All @@ -59,11 +52,4 @@ public static void setUpBeforeClass() throws Exception {
public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
13 changes: 0 additions & 13 deletions ...test/java/org/apache/hadoop/hbase/io/compress/aircompressor/TestWALCompressionSnappy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -29,10 +28,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionSnappy extends CompressedWALTestBase {
Expand All @@ -41,9 +37,6 @@ public class TestWALCompressionSnappy extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionSnappy.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
Expand All @@ -60,10 +53,4 @@ public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
13 changes: 0 additions & 13 deletions ...c/test/java/org/apache/hadoop/hbase/io/compress/aircompressor/TestWALCompressionZstd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -29,10 +28,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionZstd extends CompressedWALTestBase {
Expand All @@ -41,9 +37,6 @@ public class TestWALCompressionZstd extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionZstd.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
Expand All @@ -60,10 +53,4 @@ public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
13 changes: 0 additions & 13 deletions ...li/src/test/java/org/apache/hadoop/hbase/io/compress/brotli/TestWALCompressionBrotli.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -29,10 +28,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionBrotli extends CompressedWALTestBase {
Expand All @@ -41,9 +37,6 @@ public class TestWALCompressionBrotli extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionBrotli.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
Expand All @@ -60,10 +53,4 @@ public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
14 changes: 0 additions & 14 deletions ...sion-lz4/src/test/java/org/apache/hadoop/hbase/io/compress/lz4/TestWALCompressionLz4.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -29,10 +28,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionLz4 extends CompressedWALTestBase {
Expand All @@ -41,9 +37,6 @@ public class TestWALCompressionLz4 extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionLz4.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
Expand All @@ -59,11 +52,4 @@ public static void setUpBeforeClass() throws Exception {
public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
13 changes: 0 additions & 13 deletions ...py/src/test/java/org/apache/hadoop/hbase/io/compress/xerial/TestWALCompressionSnappy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -31,10 +30,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionSnappy extends CompressedWALTestBase {
Expand All @@ -43,9 +39,6 @@ public class TestWALCompressionSnappy extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionSnappy.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
assumeTrue(SnappyCodec.isLoaded());
Expand All @@ -63,10 +56,4 @@ public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
13 changes: 0 additions & 13 deletions ...ssion-xz/src/test/java/org/apache/hadoop/hbase/io/compress/xz/TestWALCompressionLzma.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -29,10 +28,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionLzma extends CompressedWALTestBase {
Expand All @@ -41,9 +37,6 @@ public class TestWALCompressionLzma extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionLzma.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
Expand All @@ -60,10 +53,4 @@ public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
14 changes: 0 additions & 14 deletions ...n-zstd/src/test/java/org/apache/hadoop/hbase/io/compress/zstd/TestWALCompressionZstd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseClassTestRule;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.io.compress.Compression;
import org.apache.hadoop.hbase.regionserver.wal.CompressionContext;
import org.apache.hadoop.hbase.testclassification.MediumTests;
Expand All @@ -29,10 +28,7 @@
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TestName;

@Category({ RegionServerTests.class, MediumTests.class })
public class TestWALCompressionZstd extends CompressedWALTestBase {
Expand All @@ -41,9 +37,6 @@ public class TestWALCompressionZstd extends CompressedWALTestBase {
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestWALCompressionZstd.class);

@Rule
public TestName name = new TestName();

@BeforeClass
public static void setUpBeforeClass() throws Exception {
Configuration conf = TEST_UTIL.getConfiguration();
Expand All @@ -59,11 +52,4 @@ public static void setUpBeforeClass() throws Exception {
public static void tearDown() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}

@Test
public void test() throws Exception {
TableName tableName = TableName.valueOf(name.getMethodName().replaceAll("[^a-zA-Z0-9]", "_"));
doTest(tableName);
}

}
17 changes: 17 additions & 0 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/CompressionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.hadoop.hbase.regionserver.wal;

import java.io.ByteArrayOutputStream;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
Expand Down Expand Up @@ -129,9 +130,25 @@ public void decompress(InputStream in, int inLength, byte[] outArray, int outOff
} else {
lowerIn.reset(in, inLength);
IOUtils.readFully(compressedIn, outArray, outOffset, outLength);
// if the uncompressed size was larger than the configured buffer size for the codec,
// the BlockCompressorStream will have left an extra 4 bytes hanging. This represents a size
// for the next segment, and it should be 0. See HBASE-28390
if (lowerIn.available() == 4) {
Copy link
Contributor

@apurtell apurtell Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is ugly but I don't have an alternative suggestion. At least we know both in the WAL read and HFile read cases we won't be contending with a short read, so available is going to be either 0 or 4 and otherwise we'd have had an EOFException thrown.

int remaining = rawReadInt(lowerIn);
assert remaining == 0;
}
}
}

private int rawReadInt(InputStream in) throws IOException {
Copy link
Contributor

@apurtell apurtell Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised commons-io or our own util code doesn't have something like this already.

Copy link
Contributor Author

@bbeaudreault bbeaudreault Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried looking and couldn't find one. I'll try looking again

Copy link
Contributor Author

@bbeaudreault bbeaudreault Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops I think IOUtils.readInt should work. Any other comments/concerns other than that?

Copy link
Contributor

@apurtell apurtell Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left an approving review with the rest. Thanks!

Copy link
Contributor Author

@bbeaudreault bbeaudreault Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, there is no IOUtils.readInt in commons-io. I realized that the IOUtils I was referring to (while on mobile) was a different library that we don't have a dependency on.

We have a variety of methods for reading ints from byte[] or ByteBuffer, but not for InputStream that I can find. I could read the bytes into a byte[4] and use one of the utils, but I'd rather not allocate an unnecessary byte array. I'm going to leave this alone for now.

int b1 = in.read();
int b2 = in.read();
int b3 = in.read();
int b4 = in.read();
if ((b1 | b2 | b3 | b4) < 0) throw new EOFException();
return ((b1 << 24) + (b2 << 16) + (b3 << 8) + (b4 << 0));
}

public void clear() {
if (compressedOut != null) {
try {
Expand Down
Loading