Skip to content

HBASE-26273 Force ReadType.STREAM when the user does not explicitly s… #3675

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.

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

Closed

Conversation

joshelser
Copy link
Member

…et a ReadType on the Scan for a Snapshot-based Job

HBase 2 moved over Scans to use PREAD by default instead of STREAM like
HBase 1. In the context of a MapReduce job, we can generally expect that
clients using the InputFormat (batch job) would be reading most of the
data for a job. Cater to them, but still give users who want PREAD the
ability to do so.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 6s 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.
_ master Compile Tests _
+1 💚 mvninstall 4m 2s master passed
+1 💚 compile 0m 50s master passed
+1 💚 checkstyle 0m 20s master passed
+1 💚 spotbugs 0m 46s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 45s the patch passed
+1 💚 compile 0m 48s the patch passed
+1 💚 javac 0m 48s the patch passed
-0 ⚠️ checkstyle 0m 18s hbase-mapreduce: The patch generated 6 new + 2 unchanged - 0 fixed = 8 total (was 2)
-0 ⚠️ whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
+1 💚 hadoopcheck 18m 29s Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.
+1 💚 spotbugs 0m 54s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 14s The patch does not generate ASF License warnings.
39m 55s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #3675
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
uname Linux b6a812e631b1 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / ee632bd
Default Java AdoptOpenJDK-1.8.0_282-b08
checkstyle https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-mapreduce.txt
whitespace https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/artifact/yetus-general-check/output/whitespace-eol.txt
Max. process+thread count 96 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/console
versions git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 5s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 4m 23s master passed
+1 💚 compile 0m 26s master passed
+1 💚 shadedjars 9m 5s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 19s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 4m 5s the patch passed
+1 💚 compile 0m 26s the patch passed
+1 💚 javac 0m 26s the patch passed
+1 💚 shadedjars 9m 5s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 17s the patch passed
_ Other Tests _
+1 💚 unit 14m 34s hbase-mapreduce in the patch passed.
45m 2s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #3675
Optional Tests javac javadoc unit shadedjars compile
uname Linux d3ce3b616276 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / ee632bd
Default Java AdoptOpenJDK-1.8.0_282-b08
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/testReport/
Max. process+thread count 2775 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 6s 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 _
_ master Compile Tests _
+1 💚 mvninstall 5m 6s master passed
+1 💚 compile 0m 30s master passed
+1 💚 shadedjars 9m 6s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 22s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 4m 45s the patch passed
+1 💚 compile 0m 30s the patch passed
+1 💚 javac 0m 30s the patch passed
+1 💚 shadedjars 8m 57s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 20s the patch passed
_ Other Tests _
+1 💚 unit 14m 35s hbase-mapreduce in the patch passed.
46m 33s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #3675
Optional Tests javac javadoc unit shadedjars compile
uname Linux 84296853c838 4.15.0-153-generic #160-Ubuntu SMP Thu Jul 29 06:54:29 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / ee632bd
Default Java AdoptOpenJDK-11.0.10+9
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/testReport/
Max. process+thread count 3357 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

Copy link
Contributor

@taklwu taklwu left a comment

Choose a reason for hiding this comment

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

LGTM,
[nit] if possible, can you fix the checkstyle and whitespace ?

Copy link
Contributor

@anoopsjohn anoopsjohn left a comment

Choose a reason for hiding this comment

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

+1

/**
* The {@link ReadType} which should be set on the {@link Scan} to read the HBase Snapshot, default STREAM.
*/
public static final String SNAPSHOT_INPUTFORMAT_SCANNER_READTYPE = "hbase.TableSnapshotinputFormat.scanner.readtype";
Copy link
Contributor

Choose a reason for hiding this comment

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

TableSnapshotinputFormat => TableSnapshotInputFormat ?

Copy link
Member Author

Choose a reason for hiding this comment

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

hah! Thanks for noticing.

Copy link
Contributor

@Apache9 Apache9 left a comment

Choose a reason for hiding this comment

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

I think this is an improvement so give a +1 first.

But just curious, how much we could gain from this change?

By default, we will switch to stream after reading a small amount of data, several hundreds of KBs? If we read several hundreds of MBs of data in a map reduce job, I do not think it will effect the performance too much?

@joshelser
Copy link
Member Author

joshelser commented Sep 13, 2021

By default, we will switch to stream after reading a small amount of data, several hundreds of KBs? If we read several hundreds of MBs of data in a map reduce job, I do not think it will effect the performance too much?

For a standalone Java program reading a ~5G file in a single JVM (... using the mapreduce snapshot APIs), this change improved run time from 90s to 30s. In a distributed system, it only had about 15% improvement (network became the bottleneck -- that's where HBASE-26274 came into play).

[nit] if possible, can you fix the checkstyle and whitespace ?

You bet. I hadn't looked at that yet. Thanks for calling it out!

…et a ReadType on the Scan for a Snapshot-based Job

HBase 2 moved over Scans to use PREAD by default instead of STREAM like
HBase 1. In the context of a MapReduce job, we can generally expect that
clients using the InputFormat (batch job) would be reading most of the
data for a job. Cater to them, but still give users who want PREAD the
ability to do so.
@joshelser joshelser force-pushed the 26273-snapshot-inputformat-stream branch from 1d00647 to a808a2d Compare September 13, 2021 20:38
@joshelser
Copy link
Member Author

a808a2d has the fixes requested. If QA comes back happy, I'll just merge this. No need to bother y'all for a re-review. Thanks for the eyes!

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 1s 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.
_ master Compile Tests _
+1 💚 mvninstall 3m 43s master passed
+1 💚 compile 0m 47s master passed
+1 💚 checkstyle 0m 18s master passed
+1 💚 spotbugs 0m 43s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 40s the patch passed
+1 💚 compile 0m 47s the patch passed
+1 💚 javac 0m 47s the patch passed
+1 💚 checkstyle 0m 18s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 18m 25s Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.
+1 💚 spotbugs 0m 55s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 12s The patch does not generate ASF License warnings.
38m 27s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #3675
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
uname Linux 837b06e74a8c 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / d26bcaa
Default Java AdoptOpenJDK-1.8.0_282-b08
Max. process+thread count 95 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/2/console
versions git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 26s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 4m 5s master passed
+1 💚 compile 0m 27s master passed
+1 💚 shadedjars 8m 12s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 22s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 43s the patch passed
+1 💚 compile 0m 28s the patch passed
+1 💚 javac 0m 28s the patch passed
+1 💚 shadedjars 8m 11s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 19s the patch passed
_ Other Tests _
+1 💚 unit 11m 20s hbase-mapreduce in the patch passed.
38m 40s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #3675
Optional Tests javac javadoc unit shadedjars compile
uname Linux 70f7e983688f 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / d26bcaa
Default Java AdoptOpenJDK-1.8.0_282-b08
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/2/testReport/
Max. process+thread count 3688 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/2/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 8s Docker mode activated.
-0 ⚠️ yetus 0m 2s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚 mvninstall 5m 10s master passed
+1 💚 compile 0m 31s master passed
+1 💚 shadedjars 9m 7s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 24s master passed
_ Patch Compile Tests _
+1 💚 mvninstall 4m 52s the patch passed
+1 💚 compile 0m 31s the patch passed
+1 💚 javac 0m 31s the patch passed
+1 💚 shadedjars 9m 18s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 21s the patch passed
_ Other Tests _
+1 💚 unit 14m 57s hbase-mapreduce in the patch passed.
47m 17s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #3675
Optional Tests javac javadoc unit shadedjars compile
uname Linux ed5a24165ba9 4.15.0-153-generic #160-Ubuntu SMP Thu Jul 29 06:54:29 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / d26bcaa
Default Java AdoptOpenJDK-11.0.10+9
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/2/testReport/
Max. process+thread count 3396 (vs. ulimit of 30000)
modules C: hbase-mapreduce U: hbase-mapreduce
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3675/2/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@huaxiangsun
Copy link
Contributor

By default, we will switch to stream after reading a small amount of data, several hundreds of KBs? If we read several hundreds of MBs of data in a map reduce job, I do not think it will effect the performance too much?

For a standalone Java program reading a ~5G file in a single JVM (... using the mapreduce snapshot APIs), this change improved run time from 90s to 30s. In a distributed system, it only had about 15% improvement (network became the bottleneck -- that's where HBASE-26274 came into play).

The number is impressive. For standalone Java program, is it hdfs local read with short circuit read enabled or through local tcp connection?

@joshelser
Copy link
Member Author

For standalone Java program, is it hdfs local read with short circuit read enabled or through local tcp connection?

This should have been a remote TCP connection. I ran these numbers on a multiple-node cluster. It's possible that part of the data was hosted by a local Datanode, but, if memory serves, it was largely remote reads.

I have it in a private Git repository with the steps I was doing to test. I can post it if you're curious to reproduce what I did.

@joshelser
Copy link
Member Author

Merged to master, branch-2, and branch-2.4.

@joshelser joshelser closed this Sep 13, 2021
@huaxiangsun
Copy link
Contributor

This should have been a remote TCP connection. I ran these numbers on a multiple-node cluster. It's possible that part of the data was hosted by a local Datanode, but, if memory serves, it was largely remote reads.

I have it in a private Git repository with the steps I was doing to test. I can post it if you're curious to reproduce what I did.

Thanks @joshelser. Was trying to understand how much improvement it does for regions with locality. If it is ok to share your private Git repo, I'd like to run the test on regions with 100% locality and share the numbers on the jira.

@joshelser
Copy link
Member Author

@huaxiangsun https://github.com/joshelser/stream-repro this is the rough outline of what I was doing. Pretty straightforward (hbase pe to make data in a table, take a snapshot, and a java -cp to just read all the data as one input split).

@Apache9
Copy link
Contributor

Apache9 commented Sep 14, 2021

For a standalone Java program reading a ~5G file in a single JVM (... using the mapreduce snapshot APIs), this change improved run time from 90s to 30s. In a distributed system, it only had about 15% improvement (network became the bottleneck -- that's where HBASE-26274 came into play).

It is a bit surprise to me that there could a 15% impact on performance. I was suppose that there should be little differences as we only read a very small amount of data with pread. Mind sharing more details here? Such as the HFile block size or something else? IIRC, the default config is to switch to stream after reading 4 HFile block size. And I saw you have already provide the test code, let me also take a look. Maybe we should file an issue about the performance issue with pread switching to stream.

Merged to master, branch-2, and branch-2.4.

I think we also need this on branch-2.3? It has not been EOL yet.

Thanks.

@Apache9
Copy link
Contributor

Apache9 commented Sep 14, 2021

Oh, just notice that this is for reading snapshot...

Let me take a look at the input format implementation.

@joshelser
Copy link
Member Author

Oh, just notice that this is for reading snapshot...

Yup! Just for reading HFiles directly from the filesystem in a local JVM.

I think we also need this on branch-2.3? It has not been EOL yet.

My bad. i'll apply there too.

@huaxiangsun
Copy link
Contributor

@huaxiangsun https://github.com/joshelser/stream-repro this is the rough outline of what I was doing. Pretty straightforward (hbase pe to make data in a table, take a snapshot, and a java -cp to just read all the data as one input split).

Thanks @joshelser, will report back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants