Skip to content

MINOR: Allow callers to provide known Hadoop file lengths#3678

Open
dossett wants to merge 1 commit into
apache:masterfrom
dossett:agent-known-file-length
Open

MINOR: Allow callers to provide known Hadoop file lengths#3678
dossett wants to merge 1 commit into
apache:masterfrom
dossett:agent-known-file-length

Conversation

@dossett

@dossett dossett commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

In an earlier PR @steveloughran suggested that callers who already knew the length of a file (such as Iceberg) could supply that value when opening a file and save at least on metadata lookup. This is the result of me working through that idea with codex, not sure if it's what was originally intended.

It does bump the hadoop dependency from 3.3.0 to 3.3.5. I assume that's safe, but calling it out just in case.

CODEX summary follows

Metadata-aware callers may already know a Parquet file's exact length from a catalog, manifest, or prior listing. HadoopInputFile.fromPath(Path, Configuration) currently performs getFileStatus() unconditionally, discarding that knowledge and potentially adding a remote metadata request before the file can be read.

This adds a length-aware factory that trusts the caller-provided value and passes it to Hadoop's openFile() API. The API is format-neutral, but it enables table formats and other metadata-aware readers to avoid the status lookup when they can guarantee the length. Existing path- and status-based construction retain their current behavior. This follows up on the known-length optimization discussed in #3395.

The change also raises the Hadoop baseline from 3.3.0 to 3.3.5, where FS_OPTION_OPENFILE_LENGTH was standardized and supported by S3A.

Summary:

  • Add HadoopInputFile.fromPath(Path, long, Configuration).
  • Preserve FileStatus as the preferred open hint when one is already available.
  • Otherwise pass the known length through FS_OPTION_OPENFILE_LENGTH.
  • Reject negative lengths and document that the supplied length is trusted without validation.

Testing:

  • ./mvnw -pl parquet-hadoop -Dtest=TestHadoopOpenFile test
  • ./mvnw -q -pl parquet-hadoop -DskipTests spotless:check

@dossett
dossett force-pushed the agent-known-file-length branch from d66f09a to 4efb816 Compare July 20, 2026 12:59
@dossett
dossett marked this pull request as ready for review July 20, 2026 13:01

@steveloughran steveloughran left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

commented, suggested one more test and a change to the javadocs; no production code changes

throw new IllegalArgumentException("Invalid file length: " + length);
}
FileSystem fs = path.getFileSystem(conf);
return new HadoopInputFile(fs, path, length, conf);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is the only place the new ctor is called, isn't it? so it can never be invoked with a length that is unknown.

* Creates an input file using a caller-supplied file length.
*
* <p>The length is trusted and no file status lookup is performed. Callers must provide the exact
* length of the file that will be opened. The file is not validated until it is opened.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

declare that the existence of the file may not be checked until the first read, and the length may be trusted; if it is wrong then reads may fail.

* A caller-supplied length avoids a status lookup and is passed to openFile().
*/
@Test
public void testOpenFileWithKnownLength() throws Throwable {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add a test opening a nonexistent file with a declared length. For cloud storage, it may not fail until the first read(); for local fs it will fail immediately. In both cases it must fail with FileNotFoundException.

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.

2 participants