You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by GitBox <gi...@apache.org> on 2022/08/04 16:05:13 UTC
[GitHub] [avro] steveloughran opened a new pull request, #1807: AVRO-3594. FsInput to use openFile() API
steveloughran opened a new pull request, #1807:
URL: https://github.com/apache/avro/pull/1807
Boost performance reading from object stores in hadoop 3.3.5+ by using the openFile builder API and passing in the file length as an option (can save a HEAD) and asks for adaptive IO (sequential going to random if the client starts seeking)
saving that HEAD request is a key benefit against s3 as it can save 50-100 mS per file.
### Jira
- [X] My PR addresses the following [Avro Jira](https://issues.apache.org/jira/browse/AVRO/) issues and references them in the PR title. For example, "AVRO-1234: My Avro PR"
- https://issues.apache.org/jira/browse/AVRO-XXX
- In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
### Tests
- [X] My PR does not need testing for this extremely good reason:
1. All existing local file IO tests act as regression tests.
2. avro isn't set up for integration tests with abfs/gs/s3a urls.
3. mocking doesn't really do much here.
Integration tests would be the way to do this, but the foundational set up to do this
is pretty complex. My [cloudstream](https://github.com/hortonworks-spark/cloud-integration) project downstream of spark is set up to do this.
### Commits
- [X] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](https://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [X] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain Javadoc that explain what it does
no new docs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] steveloughran commented on pull request #1807: AVRO-3594. FsInput to use openFile() API
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #1807:
URL: https://github.com/apache/avro/pull/1807#issuecomment-1206667386
@clesaec this can't go in until hadoop2 is cut as a profile.
I am actually doing a shim library to do reflection invocation on the newer operations, with fallbacks if not found.
https://github.com/steveloughran/fs-api-shim
but even so, the sooner avro goes to recent hadoop 3.x release *only* the better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] steveloughran commented on a diff in pull request #1807: AVRO-3594. FsInput to use openFile() API
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #1807:
URL: https://github.com/apache/avro/pull/1807#discussion_r944483317
##########
lang/java/mapred/src/main/java/org/apache/avro/mapred/FsInput.java:
##########
@@ -41,7 +43,15 @@ public FsInput(Path path, Configuration conf) throws IOException {
/** Construct given a path and a {@code FileSystem}. */
public FsInput(Path path, FileSystem fileSystem) throws IOException {
this.len = fileSystem.getFileStatus(path).getLen();
- this.stream = fileSystem.open(path);
+ // use the hadoop 3.3.0 openFile API and specify length
+ // and read policy. object stores can use these to
+ // optimize read performance.
+ // the read policy "adaptive" means "start sequential but
+ // go to random IO after backwards seeks"
+ // Filesystems which don't recognize the options will ignore them
+
+ this.stream = awaitFuture(fileSystem.openFile(path).opt("fs.option.openfile.read.policy", "adaptive")
+ .opt("fs.option.openfile.length", Long.toString(len)).build());
Review Comment:
note that `org.apache.hadoop.fs.AvroFSInput` does this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] clesaec commented on a diff in pull request #1807: AVRO-3594. FsInput to use openFile() API
Posted by GitBox <gi...@apache.org>.
clesaec commented on code in PR #1807:
URL: https://github.com/apache/avro/pull/1807#discussion_r938651175
##########
lang/java/mapred/src/main/java/org/apache/avro/mapred/FsInput.java:
##########
@@ -41,7 +43,15 @@ public FsInput(Path path, Configuration conf) throws IOException {
/** Construct given a path and a {@code FileSystem}. */
public FsInput(Path path, FileSystem fileSystem) throws IOException {
this.len = fileSystem.getFileStatus(path).getLen();
- this.stream = fileSystem.open(path);
+ // use the hadoop 3.3.0 openFile API and specify length
+ // and read policy. object stores can use these to
+ // optimize read performance.
+ // the read policy "adaptive" means "start sequential but
+ // go to random IO after backwards seeks"
+ // Filesystems which don't recognize the options will ignore them
+
+ this.stream = awaitFuture(fileSystem.openFile(path).opt("fs.option.openfile.read.policy", "adaptive")
+ .opt("fs.option.openfile.length", Long.toString(len)).build());
Review Comment:
ok, thanks for explanation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] steveloughran commented on pull request #1807: AVRO-3594. FsInput to use openFile() API
Posted by GitBox <gi...@apache.org>.
steveloughran commented on PR #1807:
URL: https://github.com/apache/avro/pull/1807#issuecomment-1205517072
except that hadoop-2 profile still exists, doesn't it? which means that even though hadoop 3 profile is full of features, the 2.x one blocks things from working
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] clesaec commented on a diff in pull request #1807: AVRO-3594. FsInput to use openFile() API
Posted by GitBox <gi...@apache.org>.
clesaec commented on code in PR #1807:
URL: https://github.com/apache/avro/pull/1807#discussion_r938564843
##########
lang/java/mapred/src/main/java/org/apache/avro/mapred/FsInput.java:
##########
@@ -41,7 +43,15 @@ public FsInput(Path path, Configuration conf) throws IOException {
/** Construct given a path and a {@code FileSystem}. */
public FsInput(Path path, FileSystem fileSystem) throws IOException {
this.len = fileSystem.getFileStatus(path).getLen();
- this.stream = fileSystem.open(path);
+ // use the hadoop 3.3.0 openFile API and specify length
+ // and read policy. object stores can use these to
+ // optimize read performance.
+ // the read policy "adaptive" means "start sequential but
+ // go to random IO after backwards seeks"
+ // Filesystems which don't recognize the options will ignore them
+
+ this.stream = awaitFuture(fileSystem.openFile(path).opt("fs.option.openfile.read.policy", "adaptive")
+ .opt("fs.option.openfile.length", Long.toString(len)).build());
Review Comment:
Nice optimization,
May be better with usage of Options.OpenFileOptions constants
[FS_OPTION_OPENFILE_LENGTH](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Options.java#L555-L556)
[FS_OPTION_OPENFILE_READ_POLICY](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Options.java#L579-L580)
[FS_OPTION_OPENFILE_READ_POLICY_ADAPTIVE](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Options.java#L598-L599)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [avro] steveloughran commented on a diff in pull request #1807: AVRO-3594. FsInput to use openFile() API
Posted by GitBox <gi...@apache.org>.
steveloughran commented on code in PR #1807:
URL: https://github.com/apache/avro/pull/1807#discussion_r938627811
##########
lang/java/mapred/src/main/java/org/apache/avro/mapred/FsInput.java:
##########
@@ -41,7 +43,15 @@ public FsInput(Path path, Configuration conf) throws IOException {
/** Construct given a path and a {@code FileSystem}. */
public FsInput(Path path, FileSystem fileSystem) throws IOException {
this.len = fileSystem.getFileStatus(path).getLen();
- this.stream = fileSystem.open(path);
+ // use the hadoop 3.3.0 openFile API and specify length
+ // and read policy. object stores can use these to
+ // optimize read performance.
+ // the read policy "adaptive" means "start sequential but
+ // go to random IO after backwards seeks"
+ // Filesystems which don't recognize the options will ignore them
+
+ this.stream = awaitFuture(fileSystem.openFile(path).opt("fs.option.openfile.read.policy", "adaptive")
+ .opt("fs.option.openfile.length", Long.toString(len)).build());
Review Comment:
we only added those options explicitly in branch-3.3; the code wouldn't compile/link with 3.3.0. hence the strings. Unfortunately it also means you don't get that speedup until 3.3.5 ships ....but the release will be ready.
3.3.0 did add the `withFileStatus(FileStatus)` param which was my original design for passing in all filestatus info, inc etag and maybe version, so you can go straight from listing to opening.
first in s3a, added abfs in 3.3.5. but it is too brittle because the path checking requires status.getPath to equal the path opened. and hive with its wrapper fs doesn't always do that.
Passing in file length is guaranteed to be ignored or actually used...no brittleness. it also suits hive where workers know the length of the file but don't have a status.
One thing i can add with immediate benefit in 3.3.0 is the initial the fs.s3a.experimental.fadvise option, which again can mandate be adaptive, even on hive clusters where they explicitly set read policy to be random (which some do for max orc/parquet performance). The new opt fs.option.openfile.read.policy is an evolution of that (you can now specify a list of policies and the first one recognised is understood. if someone ever implemented explicit "parquet", "orc", and "avro" for example), you could say the read policy is "orc, random, adaptive" and get the first one known.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org