HADOOP-19793. S3A: use long for file size in S3A content providers, data blocks

hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml

@@ -86,4 +86,13 @@
     <Method name="submit"/>
     <Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE"/>
   </Match>
+  <!-- Despite adding `final` as suggested, spotbugs kept complaining. -->
+  <Match>
+    <Class name="org.apache.hadoop.fs.s3a.commit.files.PendingSet"/>
+    <Bug pattern="MC_OVERRIDABLE_METHOD_CALL_IN_READ_OBJECT"/>
+  </Match>
+  <Match>
+    <Class name="org.apache.hadoop.fs.s3a.commit.files.SinglePendingCommit"/>
+    <Bug pattern="MC_OVERRIDABLE_METHOD_CALL_IN_READ_OBJECT"/>
+  </Match>
 </FindBugsFilter>

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ADataBlocks.java

@@ -133,9 +133,8 @@ public UploadContentProviders.BaseContentProvider<?> getContentProvider() {
     public BlockUploadData(File file, final Supplier<Boolean> isOpen) {
       checkArgument(file.exists(), "No file: " + file);
       final long length = file.length();
-      checkArgument(length <= Integer.MAX_VALUE,
-          "File %s is too long to upload: %d", file, length);
-      this.contentProvider = fileContentProvider(file, 0, (int) length, isOpen);
+      checkArgument(length >= 0, "Length is negative for file %s (%d)", file, length);
+      this.contentProvider = fileContentProvider(file, 0, length, isOpen);
     }
 
     /**
@@ -169,7 +168,7 @@ public BlockUploadData(byte[] bytes, final Supplier<Boolean> isOpen) {
      * Size as declared by the content provider.
      * @return size of the data
      */
-    int getSize() {
+    long getSize() {
       return contentProvider.getSize();
     }
 

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/files/PendingSet.java

@@ -170,7 +170,7 @@ private void readObject(ObjectInputStream inStream) throws IOException,
    * Validate the data: those fields which must be non empty, must be set.
    * @throws ValidationFailure if the data is invalid
    */
-  public void validate() throws ValidationFailure {
+  public final void validate() throws ValidationFailure {
     verify(version == VERSION, "Wrong version: %s", version);
     validateCollectionClass(extraData.keySet(), String.class);
     validateCollectionClass(extraData.values(), String.class);

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/files/SinglePendingCommit.java

@@ -70,7 +70,7 @@
 @SuppressWarnings("unused")
 @InterfaceAudience.Private
 @InterfaceStability.Unstable
-public class SinglePendingCommit extends PersistentCommitData<SinglePendingCommit>
+public final class SinglePendingCommit extends PersistentCommitData<SinglePendingCommit>
     implements Iterable<UploadEtag> {
 
   /**

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/UploadContentProviders.java

@@ -88,7 +88,7 @@ public static BaseContentProvider<BufferedInputStream> fileContentProvider(
   public static BaseContentProvider<BufferedInputStream> fileContentProvider(
       File file,
       long offset,
-      final int size,
+      final long size,
       final Supplier<Boolean> isOpen) {
 
     return new FileWithOffsetContentProvider(file, offset, size, isOpen);
@@ -203,7 +203,7 @@ public static abstract class BaseContentProvider<T extends InputStream>
     /**
      * Size of the data.
      */
-    private final int size;
+    private final long size;
 
     /**
      * Probe to check if the stream is open.
@@ -235,7 +235,7 @@ public static abstract class BaseContentProvider<T extends InputStream>
      * Constructor.
      * @param size size of the data. Must be non-negative.
      */
-    protected BaseContentProvider(int size) {
+    protected BaseContentProvider(long size) {
       this(size, null);
     }
 
@@ -244,7 +244,7 @@ protected BaseContentProvider(int size) {
      * @param size size of the data. Must be non-negative.
      * @param isOpen optional predicate to check if the stream is open.
      */
-    protected BaseContentProvider(int size, @Nullable Supplier<Boolean> isOpen) {
+    protected BaseContentProvider(long size, @Nullable Supplier<Boolean> isOpen) {
       checkArgument(size >= 0, "size is negative: %s", size);
       this.size = size;
       this.isOpen = isOpen;
@@ -311,7 +311,7 @@ public int getStreamCreationCount() {
      * Size as set by constructor parameter.
      * @return size of the data
      */
-    public int getSize() {
+    public long getSize() {
       return size;
     }
 
@@ -385,11 +385,11 @@ private static final class FileWithOffsetContentProvider
     private FileWithOffsetContentProvider(
         final File file,
         final long offset,
-        final int size,
+        final long size,
         @Nullable final Supplier<Boolean> isOpen) {
       super(size, isOpen);
       this.file = requireNonNull(file);
-      checkArgument(offset >= 0, "Offset is negative: %s", offset);
+      checkArgument(offset >= 0, "Offset is negative: %d", offset);
       this.offset = offset;
     }
 
@@ -402,7 +402,7 @@ private FileWithOffsetContentProvider(
      */
     private FileWithOffsetContentProvider(final File file,
         final long offset,
-        final int size) {
+        final long size) {
       this(file, offset, size, null);
     }
 
@@ -482,7 +482,10 @@ protected ByteBufferInputStream createNewStream() {
       // set the buffer up from reading from the beginning
       blockBuffer.limit(initialPosition);
       blockBuffer.position(0);
-      return new ByteBufferInputStream(getSize(), blockBuffer);
+      long size = getSize();
+      // Shouldn't happen since constructor takes an int.
+      checkState(size <= Integer.MAX_VALUE, "Size too large for ByteBufferInputStream: %s", size);
+      return new ByteBufferInputStream((int)size, blockBuffer);
     }
 
     @Override
@@ -546,7 +549,7 @@ private ByteArrayContentProvider(
       super(size, isOpen);
       this.bytes = bytes;
       this.offset = offset;
-      checkArgument(offset >= 0, "Offset is negative: %s", offset);
+      checkArgument(offset >= 0, "Offset is negative: %d", offset);
       final int length = bytes.length;
       checkArgument((offset + size) <= length,
           "Data to read [%d-%d] is past end of array %s",
@@ -556,7 +559,11 @@ private ByteArrayContentProvider(
 
     @Override
     protected ByteArrayInputStream createNewStream() {
-      return new ByteArrayInputStream(bytes, offset, getSize());
+      long size = getSize();
+      // Shouldn't happen since constructor takes an int.
+      checkState(size <= Integer.MAX_VALUE,
+          "Size too large for ByteArrayContentProvider: %d", size);
+      return new ByteArrayInputStream(bytes, offset, (int)size);
     }
 
     @Override

hadoop-tools/hadoop-azure/src/config/checkstyle.xml

@@ -169,7 +169,9 @@
         <module name="FinalClass"/>
         <module name="HideUtilityClassConstructor"/>
         <module name="InterfaceIsType"/>
-        <module name="VisibilityModifier"/>
+        <module name="VisibilityModifier">
+            <property name="protectedAllowed" value="true"/>
+        </module>
 
 
         <!-- Miscellaneous other checks.                   -->