HBASE-28065 Corrupt HFile data is mishandled in several cases (branch-2.5) by ndimiduk · Pull Request #5384 · apache/hbase

ndimiduk · 2023-09-06T14:16:02Z

when no block size is provided and there's not a preread headerBuf, treat the value with caution.
verify HBase checksums before making use of the block header.
inline verifyOnDiskSizeMatchesHeader to keep throw/return logic in the method body.
whenever a read is determined to be corrupt and fallback to HDFS checksum is necessary, also invalidate the cached value of headerBuf.
built out a test suite covering various forms of block header corruption, for blocks in first and second positions.

ndimiduk · 2023-09-08T15:29:42Z

Added a new unit test that relies on a LocalFileSystem to mess with the hfile under the covers.

ndimiduk · 2023-09-11T12:34:46Z

+ HFile.Reader reader = null;
+ HFileBlock.BlockIterator iter = null;
+ try {
+ reader = HFile.createReader(hfs, hfsPath, CacheConfig.DISABLED, true, conf);


It seems like there's a couple ways to read through an HFile -- this is roughly how the HFilePrettyPrinter does it. Is this how we want to do it in this test?

ndimiduk · 2023-09-11T12:36:01Z

+ }
+
+ /**
+ * Enables writing and rewriting portions of the file backing an {@link HFileBlock}.


This implementation assumes a LocalFileSystem. I haven't yet looked at how to corrupt a header underneath a data node.

ndimiduk · 2023-09-11T12:37:36Z

+ }
+
+ @Test
+ public void testBlockMagicCorruptionFirstBlock() throws Exception {


First and second block access patterns are slightly different because the caller has slightly more block location data due to the next-header over-read. Thus they have different failure modes ; thus they're tested in two different test methods.

ndimiduk · 2023-09-11T12:39:50Z

+ }
+
+ @Test
+ public void testUncompressedSizeWithoutHeaderCorruption() throws Exception {


I stubbed out test methods for all the Header fields, but, if we're careful, we don't need to test all of them because only a couple fields are read before the header checksum has been validated. Once the dust settles, I'll comment the test class with notes about which headers are tested and why, dropping the rest.

ndimiduk · 2023-09-11T12:40:42Z

+ byte[] family = Bytes.toBytes("f");
+ try (HFile.Writer writer = factory.create()) {
+ for (int i = 0; i < 40; i++) {
+ byte[] row = RandomKeyValueUtil.randomOrderedFixedLengthKey(rand, i, 100);


All this could be encapsulated in a RandomCellGenerator kind of class.

ndimiduk · 2023-09-11T12:51:15Z

Looks like HEAD was broken by #5379.

charlesconnell

To the extent that I understand the issues here, this seems good to me.

bbeaudreault

looks great! I have a few small comments/questions just to dot all of our i's

ndimiduk · 2023-09-13T06:40:42Z

https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/6/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt

Checkstyle warning is,

./hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:1675: protected HFileBlock readBlockDataInternal(FSDataInputStream is, long offset,:5: Method length is 154 lines (max allowed is 150). [MethodLength]

It's counting the entire body of the method, which is comprised more of line comments than code, in that tally.

ndimiduk · 2023-09-13T06:46:10Z

The test failed due to an IllegalArgumentException that had no message? I wish for an ExceptionMatcher class that prints the exception's stacktrace in the failure description.

java.lang.AssertionError: Expected: a string starting with "newLimit > capacity"
but: was null
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:8)
at org.apache.hadoop.hbase.io.hfile.TestHFileBlockHeaderCorruption.testOnDiskSizeWithoutHeaderCorruptionFirstBlock(TestHFileBlockHeaderCorruption.java:127)

ndimiduk · 2023-09-13T06:48:30Z

This one is identical, I just missed converting the asserts to use matchers -- a null for the IllegalArgumentException exception message.

Stacktrace
java.lang.NullPointerException
at org.apache.hadoop.hbase.io.hfile.TestHFileBlockHeaderCorruption.testOnDiskSizeWithoutHeaderCorruptionSecondBlock(TestHFileBlockHeaderCorruption.java:198)

ndimiduk · 2023-09-13T06:52:11Z

Hmm the test run on #5398 failed identically.

Apache-HBase · 2023-09-19T08:45:32Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 36s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	2m 31s	branch-2.5 passed
+1 💚	compile	2m 24s	branch-2.5 passed
+1 💚	checkstyle	0m 34s	branch-2.5 passed
+1 💚	spotless	0m 40s	branch has no errors when running spotless:check.
+1 💚	spotbugs	1m 24s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 29s	the patch passed
+1 💚	compile	2m 19s	the patch passed
+1 💚	javac	2m 19s	the patch passed
-0 ⚠️	checkstyle	0m 34s	hbase-server: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	12m 36s	Patch does not cause any errors with Hadoop 2.10.2 or 3.2.4 3.3.6.
+1 💚	spotless	0m 40s	patch has no errors when running spotless:check.
+1 💚	spotbugs	1m 32s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 10s	The patch does not generate ASF License warnings.
		29m 57s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname	Linux 4d7a1bfeb58c 5.4.0-152-generic #169-Ubuntu SMP Tue Jun 6 22:23:09 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Eclipse Adoptium-11.0.17+8
checkstyle	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt
Max. process+thread count	78 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/console
versions	git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-09-19T08:52:43Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 54s	Docker mode activated.
-0 ⚠️	yetus	0m 6s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	3m 7s	branch-2.5 passed
+1 💚	compile	0m 48s	branch-2.5 passed
+1 💚	shadedjars	5m 23s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 24s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 33s	the patch passed
+1 💚	compile	0m 54s	the patch passed
+1 💚	javac	0m 54s	the patch passed
+1 💚	shadedjars	5m 42s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 24s	the patch passed
		_ Other Tests _
-1 ❌	unit	15m 10s	hbase-server in the patch failed.
		37m 0s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 4031afdd3a86 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Temurin-1.8.0_352-b08
unit	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/artifact/yetus-jdk8-hadoop2-check/output/patch-unit-hbase-server.txt
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/testReport/
Max. process+thread count	1720 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-09-19T12:18:40Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 41s	Docker mode activated.
-0 ⚠️	yetus	0m 5s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	3m 14s	branch-2.5 passed
+1 💚	compile	0m 46s	branch-2.5 passed
+1 💚	shadedjars	5m 11s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 23s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 40s	the patch passed
+1 💚	compile	0m 44s	the patch passed
+1 💚	javac	0m 44s	the patch passed
+1 💚	shadedjars	5m 11s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 23s	the patch passed
		_ Other Tests _
+1 💚	unit	219m 41s	hbase-server in the patch passed.
		243m 0s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 672ce046cc09 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Eclipse Adoptium-11.0.17+8
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/testReport/
Max. process+thread count	4643 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/10/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

ndimiduk · 2023-09-19T13:02:51Z

I think that the test failure is from a difference in JDK implementation -- Jenkins is running Eclipse Adoptium-11.0.17+8 while I'm testing on Eclipse Adoptium-11.0.18+10. Both versions throw an IllegalArgumentException, but the portions of the stack traces within the jvm code differ a bit, and indeed the one on Jenkins does not include a message in the exception. I'm downloading the older version of the JDK to see if I can reproduce the failure. Meanwhile, I didn't realize this detail of the exception came up from the JDK ; let me make the test less specific to the runtime.

ndimiduk · 2023-09-19T13:40:01Z

For reference, the two stack traces look like,

java.lang.IllegalArgumentException
at java.nio.Buffer.limit(Buffer.java:275)
at org.apache.hadoop.hbase.nio.SingleByteBuff.limit(SingleByteBuff.java:111)
at org.apache.hadoop.hbase.nio.SingleByteBuff.limit(SingleByteBuff.java:37)
at org.apache.hadoop.hbase.io.hfile.ChecksumUtil.validateChecksum(ChecksumUtil.java:184)
at org.apache.hadoop.hbase.io.hfile.HFileBlock$FSReaderImpl.validateChecksum(HFileBlock.java:1867)
at org.apache.hadoop.hbase.io.hfile.HFileBlock$FSReaderImpl.readBlockDataInternal(HFileBlock.java:1769)
at org.apache.hadoop.hbase.io.hfile.HFileBlock$FSReaderImpl.readBlockData(HFileBlock.java:1519)
at org.apache.hadoop.hbase.io.hfile.HFileBlock$FSReaderImpl$1.nextBlock(HFileBlock.java:1410)
at org.apache.hadoop.hbase.io.hfile.TestHFileBlockHeaderCorruption$HFileBlockChannelPositionIterator.hasNext(TestHFileBlockHeaderCorruption.java:349)
at org.apache.hadoop.hbase.io.hfile.TestHFileBlockHeaderCorruption$CountingConsumer.readFully(TestHFileBlockHeaderCorruption.java:307)
at org.apache.hadoop.hbase.io.hfile.TestHFileBlockHeaderCorruption.testOnDiskSizeWithoutHeaderCorruptionFirstBlock(TestHFileBlockHeaderCorruption.java:132)

vs.

java.lang.IllegalArgumentException: newLimit > capacity: (4378 > 66)
at java.base/java.nio.Buffer.createLimitException(Buffer.java:395)
at java.base/java.nio.Buffer.limit(Buffer.java:369)
at java.base/java.nio.ByteBuffer.limit(ByteBuffer.java:1529)
at java.base/java.nio.ByteBuffer.limit(ByteBuffer.java:267)
at org.apache.hadoop.hbase.nio.SingleByteBuff.limit(SingleByteBuff.java:111)
at org.apache.hadoop.hbase.nio.SingleByteBuff.limit(SingleByteBuff.java:37)
at org.apache.hadoop.hbase.io.hfile.ChecksumUtil.validateChecksum(ChecksumUtil.java:184)
at org.apache.hadoop.hbase.io.hfile.HFileBlock$FSReaderImpl.validateChecksum(HFileBlock.java:1867)
at org.apache.hadoop.hbase.io.hfile.HFileBlock$FSReaderImpl.readBlockDataInternal(HFileBlock.java:1769)
at org.apache.hadoop.hbase.io.hfile.HFileBlock$FSReaderImpl.readBlockData(HFileBlock.java:1519)
at org.apache.hadoop.hbase.io.hfile.HFileBlock$FSReaderImpl$1.nextBlock(HFileBlock.java:1410)
at org.apache.hadoop.hbase.io.hfile.TestHFileBlockHeaderCorruption$HFileBlockChannelPositionIterator.hasNext(TestHFileBlockHeaderCorruption.java:350)
at org.apache.hadoop.hbase.io.hfile.TestHFileBlockHeaderCorruption$CountingConsumer.readFully(TestHFileBlockHeaderCorruption.java:308)
at org.apache.hadoop.hbase.io.hfile.TestHFileBlockHeaderCorruption.testOnDiskSizeWithoutHeaderCorruptionFirstBlock(TestHFileBlockHeaderCorruption.java:132)

ndimiduk · 2023-09-19T13:53:41Z

Wait, the failing stack trace doesn't include module information in the JDK path entires. Was that faliing test run on JDK8?

Apache-HBase · 2023-09-19T14:16:19Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 38s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 1s	The patch does not contain any @author tags.
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	2m 31s	branch-2.5 passed
+1 💚	compile	2m 22s	branch-2.5 passed
+1 💚	checkstyle	0m 37s	branch-2.5 passed
+1 💚	spotless	0m 43s	branch has no errors when running spotless:check.
+1 💚	spotbugs	1m 23s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 28s	the patch passed
+1 💚	compile	2m 18s	the patch passed
+1 💚	javac	2m 18s	the patch passed
-0 ⚠️	checkstyle	0m 33s	hbase-server: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	12m 39s	Patch does not cause any errors with Hadoop 2.10.2 or 3.2.4 3.3.6.
+1 💚	spotless	0m 42s	patch has no errors when running spotless:check.
+1 💚	spotbugs	1m 34s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 10s	The patch does not generate ASF License warnings.
		30m 21s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname	Linux 3cb07efe704b 5.4.0-152-generic #169-Ubuntu SMP Tue Jun 6 22:23:09 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Eclipse Adoptium-11.0.17+8
checkstyle	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt
Max. process+thread count	79 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/console
versions	git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-09-19T17:59:56Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 51s	Docker mode activated.
-0 ⚠️	yetus	0m 6s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	3m 24s	branch-2.5 passed
+1 💚	compile	0m 54s	branch-2.5 passed
+1 💚	shadedjars	5m 57s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 28s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	3m 19s	the patch passed
+1 💚	compile	0m 54s	the patch passed
+1 💚	javac	0m 54s	the patch passed
+1 💚	shadedjars	6m 2s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 30s	the patch passed
		_ Other Tests _
+1 💚	unit	227m 37s	hbase-server in the patch passed.
		254m 4s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 5ed9e42910ea 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Eclipse Adoptium-11.0.17+8
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/testReport/
Max. process+thread count	4935 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-09-19T18:43:10Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 44s	Docker mode activated.
-0 ⚠️	yetus	0m 5s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	3m 6s	branch-2.5 passed
+1 💚	compile	0m 46s	branch-2.5 passed
+1 💚	shadedjars	5m 23s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 24s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 33s	the patch passed
+1 💚	compile	0m 54s	the patch passed
+1 💚	javac	0m 54s	the patch passed
+1 💚	shadedjars	5m 35s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 26s	the patch passed
		_ Other Tests _
-1 ❌	unit	272m 48s	hbase-server in the patch failed.
		297m 11s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux c7a5699b1624 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Temurin-1.8.0_352-b08
unit	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/artifact/yetus-jdk8-hadoop2-check/output/patch-unit-hbase-server.txt
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/testReport/
Max. process+thread count	4871 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/11/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

bbeaudreault

Good find on the jvm test issue. LGTM

Also of note, we've deployed this on about 90% of our fleet

ndimiduk · 2023-09-20T08:04:59Z

Failures on java8 look unrelated. The new tests were run and passed on both JVM environments.

* when no block size is provided and there's not a preread headerBuf, treat the value with caution. * verify HBase checksums before making use of the block header. * inline verifyOnDiskSizeMatchesHeader to keep throw/return logic in the method body. * separate validation of onDiskSizeWithHeader as input parameter from as read from block header * simplify branching around fetching and populating onDiskSizeWithHeader. * inline retrieving nextOnDiskBlockSize ; add basic validation. * whenever a read is determined to be corrupt and fallback to HDFS checksum is necessary, also invalidate the cached value of headerBuf. * build out a test suite covering various forms of block header corruption, for blocks in first and second positions. Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>

Apache-HBase · 2023-09-20T10:01:09Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 38s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	hbaseanti	0m 0s	Patch does not have any anti-patterns.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	2m 28s	branch-2.5 passed
+1 💚	compile	2m 20s	branch-2.5 passed
+1 💚	checkstyle	0m 32s	branch-2.5 passed
+1 💚	spotless	0m 40s	branch has no errors when running spotless:check.
+1 💚	spotbugs	1m 23s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 27s	the patch passed
+1 💚	compile	2m 18s	the patch passed
+1 💚	javac	2m 18s	the patch passed
-0 ⚠️	checkstyle	0m 35s	hbase-server: The patch generated 1 new + 3 unchanged - 0 fixed = 4 total (was 3)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	hadoopcheck	12m 46s	Patch does not cause any errors with Hadoop 2.10.2 or 3.2.4 3.3.6.
+1 💚	spotless	0m 40s	patch has no errors when running spotless:check.
+1 💚	spotbugs	1m 35s	the patch passed
		_ Other Tests _
+1 💚	asflicense	0m 10s	The patch does not generate ASF License warnings.
		30m 24s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/artifact/yetus-general-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname	Linux dcfb612fe1d4 5.4.0-152-generic #169-Ubuntu SMP Tue Jun 6 22:23:09 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Eclipse Adoptium-11.0.17+8
checkstyle	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt
Max. process+thread count	77 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/console
versions	git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-09-20T13:12:05Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 38s	Docker mode activated.
-0 ⚠️	yetus	0m 6s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	2m 20s	branch-2.5 passed
+1 💚	compile	0m 39s	branch-2.5 passed
+1 💚	shadedjars	4m 13s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 25s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	2m 7s	the patch passed
+1 💚	compile	0m 39s	the patch passed
+1 💚	javac	0m 39s	the patch passed
+1 💚	shadedjars	4m 16s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 23s	the patch passed
		_ Other Tests _
+1 💚	unit	201m 6s	hbase-server in the patch passed.
		221m 24s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 7a4001b19dd4 5.4.0-152-generic #169-Ubuntu SMP Tue Jun 6 22:23:09 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Temurin-1.8.0_352-b08
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/testReport/
Max. process+thread count	4299 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Apache-HBase · 2023-09-20T13:36:21Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 49s	Docker mode activated.
-0 ⚠️	yetus	0m 6s	Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
		_ Prechecks _
		_ branch-2.5 Compile Tests _
+1 💚	mvninstall	3m 32s	branch-2.5 passed
+1 💚	compile	1m 1s	branch-2.5 passed
+1 💚	shadedjars	6m 15s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 30s	branch-2.5 passed
		_ Patch Compile Tests _
+1 💚	mvninstall	3m 24s	the patch passed
+1 💚	compile	0m 58s	the patch passed
+1 💚	javac	0m 58s	the patch passed
+1 💚	shadedjars	6m 16s	patch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 27s	the patch passed
		_ Other Tests _
+1 💚	unit	217m 48s	hbase-server in the patch passed.
		245m 26s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR	#5384
Optional Tests	javac javadoc unit shadedjars compile
uname	Linux 3043e1151a06 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/hbase-personality.sh
git revision	branch-2.5 / `482548f`
Default Java	Eclipse Adoptium-11.0.17+8
Test Results	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/testReport/
Max. process+thread count	4474 (vs. ulimit of 30000)
modules	C: hbase-server U: hbase-server
Console output	https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5384/12/console
versions	git=2.34.1 maven=3.8.6
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

This comment was marked as outdated.
Sign in to view

bbeaudreault reviewed Sep 7, 2023
View reviewed changes

Comment threadhbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java Outdated

bbeaudreault reviewed Sep 7, 2023
View reviewed changes

Comment threadhbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java Outdated

bbeaudreault reviewed Sep 7, 2023
View reviewed changes

Comment threadhbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java

ndimiduk force-pushed the 28065-hfile-corrupt-reads-branch-2.5 branch from 4e60a26 to 0457e83Compare September 8, 2023 15:27

This comment was marked as outdated.
Sign in to view

ndimiduk commented Sep 11, 2023
View reviewed changes

ndimiduk force-pushed the 28065-hfile-corrupt-reads-branch-2.5 branch from 0457e83 to e68f2daCompare September 11, 2023 12:49

This comment was marked as outdated.
Sign in to view

charlesconnell reviewed Sep 11, 2023
View reviewed changes

ndimiduk force-pushed the 28065-hfile-corrupt-reads-branch-2.5 branch from 202af84 to d0b6988Compare September 12, 2023 11:49

This comment was marked as outdated.
Sign in to view

bbeaudreault reviewed Sep 12, 2023
View reviewed changes

This comment was marked as outdated.
Sign in to view

ndimiduk marked this pull request as ready for review September 12, 2023 15:02

ndimiduk requested review from anoopsjohn, apurtell, bbeaudreault, charlesconnell and chrajeshbabu and removed request for charlesconnell September 12, 2023 15:02

This comment was marked as outdated.
Sign in to view

ndimiduk force-pushed the 28065-hfile-corrupt-reads-branch-2.5 branch from 9254e9d to 2985bfaCompare September 19, 2023 08:09

ndimiduk force-pushed the 28065-hfile-corrupt-reads-branch-2.5 branch from 2985bfa to cb061b0Compare September 19, 2023 13:34

bbeaudreault approved these changes Sep 19, 2023
View reviewed changes

ndimiduk force-pushed the 28065-hfile-corrupt-reads-branch-2.5 branch from cb061b0 to aace0a4Compare September 20, 2023 09:24

ndimiduk merged commit df2328a into apache:branch-2.5Sep 21, 2023

ndimiduk deleted the 28065-hfile-corrupt-reads-branch-2.5 branch September 21, 2023 10:08

Uh oh!

Conversation

ndimiduk commented Sep 6, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

ndimiduk commented Sep 8, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

ndimidukSep 11, 2023

Choose a reason for hiding this comment

Uh oh!

ndimidukSep 11, 2023

Choose a reason for hiding this comment

Uh oh!

ndimidukSep 11, 2023

Choose a reason for hiding this comment

Uh oh!

ndimidukSep 11, 2023

Choose a reason for hiding this comment

Uh oh!

ndimidukSep 11, 2023

Choose a reason for hiding this comment

Uh oh!

ndimiduk commented Sep 11, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

charlesconnell left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

bbeaudreault left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ndimiduk commented Sep 13, 2023

Uh oh!

ndimiduk commented Sep 13, 2023

Uh oh!

ndimiduk commented Sep 13, 2023

Uh oh!

ndimiduk commented Sep 13, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Apache-HBase commented Sep 19, 2023

Uh oh!

Apache-HBase commented Sep 19, 2023

Uh oh!

Apache-HBase commented Sep 19, 2023

Uh oh!

ndimiduk commented Sep 19, 2023

Uh oh!

ndimiduk commented Sep 19, 2023

Uh oh!

ndimiduk commented Sep 19, 2023

Uh oh!

Apache-HBase commented Sep 19, 2023

Uh oh!

ndimiduk commented Sep 6, 2023•
edited
Loading