Adding support for readFully API #832
Conversation
|
/gcbrun |
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFSInputStream.java
Show resolved
Hide resolved
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFSInputStream.java
Outdated
Show resolved
Hide resolved
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFSInputStream.java
Outdated
Show resolved
Hide resolved
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFSInputStream.java
Show resolved
Hide resolved
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFSInputStream.java
Show resolved
Hide resolved
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFSInputStream.java
Show resolved
Hide resolved
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFSInputStream.java
Show resolved
Hide resolved
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFSInputStream.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #832 +/- ##
============================================
+ Coverage 80.40% 80.43% +0.02%
Complexity 1986 1986
============================================
Files 136 136
Lines 8989 9012 +23
Branches 1040 1042 +2
============================================
+ Hits 7228 7249 +21
- Misses 1344 1346 +2
Partials 417 417
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| in.readFully(2, value); | ||
| assertThat(in.getPos()).isEqualTo(0); | ||
| } | ||
| assertThat(value).isEqualTo(expected); |
There was a problem hiding this comment.
May you add test that demonstrates impact of this change (we have tests that assert number of GCS requests). For example, do we expect that number of the HTTP JSON GCS requests for readFully calls will be reduced after this change?
| @@ -124,6 +122,35 @@ public synchronized int read(@Nonnull byte[] buf, int offset, int length) throws | |||
| return response; | |||
| } | |||
|
|
|||
| @Override | |||
| public void readFully(long position, byte[] buffer, int offset, int length) throws IOException { | |||
There was a problem hiding this comment.
PR comment mentions that this allows to avoid unnecessary seek. May you also clarify why is this important? Is this because we are sending new GCS request for each seek?
FSInputStream exposes readFully API to read entire content of file. With this implementation, we avoid unnecessary seeks while reading entire content.