You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by fa...@apache.org on 2026/03/11 17:27:03 UTC
(hadoop) branch branch-3.5 updated: HADOOP-19793. S3A: use long for file size in content providers, data blocks (#8225) (#8339)
This is an automated email from the ASF dual-hosted git repository.
fabbri pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/branch-3.5 by this push:
new 09d98883a5b HADOOP-19793. S3A: use long for file size in content providers, data blocks (#8225) (#8339)
09d98883a5b is described below
commit 09d98883a5b41f21747808629e770b2a351afc22
Author: Aaron Fabbri <fa...@apache.org>
AuthorDate: Wed Mar 11 10:26:57 2026 -0700
HADOOP-19793. S3A: use long for file size in content providers, data blocks (#8225) (#8339)
* HADOOP-19793: use long for file size in S3A content providers, data blocks
* s3a: fix spotbugs warning..
* hadoop-aws: allow protected fields
* Spotbugs: Surpresses an existing warning that my edit re-triggered.
* Disabled checkstyle visibility modifier check for all of hadoop-aws--existing code doesn't pass and it is of questionable value.
---
.../hadoop-aws/dev-support/findbugs-exclude.xml | 9 +++++++
.../org/apache/hadoop/fs/s3a/S3ADataBlocks.java | 7 +++---
.../hadoop/fs/s3a/commit/files/PendingSet.java | 2 +-
.../fs/s3a/commit/files/SinglePendingCommit.java | 2 +-
.../hadoop/fs/s3a/impl/UploadContentProviders.java | 29 ++++++++++++++--------
.../hadoop-azure/src/config/checkstyle.xml | 4 ++-
6 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml b/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml
index 11410a24c1a..91d1d7aa0e5 100644
--- a/hadoop-tools/hadoop-aws/dev-support/findbugs-exclude.xml
+++ b/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>
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ADataBlocks.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ADataBlocks.java
index dff7493e08b..35b415e32c9 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ADataBlocks.java
+++ b/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();
}
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/files/PendingSet.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/files/PendingSet.java
index 108e6d45bc6..66d5958206e 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/files/PendingSet.java
+++ b/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);
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/files/SinglePendingCommit.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/files/SinglePendingCommit.java
index aaa91bc757c..8cda80131cf 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/commit/files/SinglePendingCommit.java
+++ b/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> {
/**
diff --git a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/UploadContentProviders.java b/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/UploadContentProviders.java
index 9716a09158e..479f6dc9a97 100644
--- a/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/UploadContentProviders.java
+++ b/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
diff --git a/hadoop-tools/hadoop-azure/src/config/checkstyle.xml b/hadoop-tools/hadoop-azure/src/config/checkstyle.xml
index 627bb65a28b..38ca2e01657 100644
--- a/hadoop-tools/hadoop-azure/src/config/checkstyle.xml
+++ b/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. -->
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org