Skip to content

HDDS-15877. Extend getFileStatus head-op (type-only) optimization to o3fs and remaining type-only callers#10779

Open
jojochuang wants to merge 1 commit into
apache:masterfrom
jojochuang:HDDS-15877
Open

HDDS-15877. Extend getFileStatus head-op (type-only) optimization to o3fs and remaining type-only callers#10779
jojochuang wants to merge 1 commit into
apache:masterfrom
jojochuang:HDDS-15877

Conversation

@jojochuang

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Follow-up of HDDS-15678, which introduced a headOp (metadata / type-only) flag so that OFS (ofs://) isDirectory/isFile skip the SCM pipeline refresh (an OM→SCM round-trip) and block-location retrieval — those calls only need the entry type, not block locations.

That change only covered the rooted (ofs://) filesystem. This PR extends the same optimization to the remaining call sites that fetch a full FileStatus solely to determine whether a path is a file or a directory:

  1. o3fs BasicOzoneFileSystem.isDirectory/isFile (primary). These still delegated to the deprecated FileSystem.super.isDirectory/isFile, which internally call the full getFileStatus(f) and therefore trigger a pipeline refresh for file paths. They now go through a head-op getFileStatus, mirroring what HDDS-15678 did for the rooted filesystem. BasicOzoneClientAdapterImpl gains a headOp getFileStatus override that forwards the flag to OzoneBucket.getFileStatus(key, headOp). Without this, o3fs kept paying the SCM round-trip for isDirectory/isFile while ofs did not. The @SuppressWarnings("deprecation") (previously needed for the super.* calls) is removed as those calls are gone.
  2. DeleteKeyHandler (ozone sh key delete on FSO buckets). The two bucket.getFileStatus(...).isDirectory() checks now pass headOp=true (the API added by HDDS-15678).
  3. Trash-root type checks in BasicOzoneFileSystem#getTrashRoots and BasicRootedOzoneClientAdapterImpl#getTrashRoots. These fetched the same path's status via exists() plus up to two getFileStatus() calls; they now fetch it once. (Trash roots are directories, which never triggered a pipeline refresh, so the win here is removing redundant RPCs rather than skipping a refresh.)

Behaviour is unchanged: isDirectory/isFile still return false for a non-existent path (FileNotFoundException is swallowed) and still propagate other IOExceptions.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15877

How was this patch tested?

  • New unit tests in ozonefs-common:
    • TestBasicOzoneClientAdapterHeadOp — verifies the o3fs adapter forwards headOp into OzoneBucket.getFileStatus, that the 4-arg overload defaults to headOp=false, and that FILE_NOT_FOUND maps to FileNotFoundException while other OMExceptions propagate.
    • TestOzoneFileSystemHeadOp — verifies BasicOzoneFileSystem.isDirectory/isFile request headOp=true, that getFileStatus(Path) still uses headOp=false, that missing paths return false, and that non-not-found exceptions propagate.
  • Existing ozonefs-common unit suite passes (46 tests), including the HDDS-15678 rooted head-op tests.
  • mvn checkstyle:check clean on ozone-filesystem-common and ozone-cli-shell.
  • ozone-filesystem-hadoop2 builds against the Hadoop 2.10.2 classpath (no Hadoop 3-only API used).

Made with Cursor

…o3fs and remaining type-only callers

Follow-up of HDDS-15678, which added a headOp (metadata/type-only) flag so ofs
isDirectory/isFile skip the SCM pipeline refresh and block-location retrieval.

- o3fs BasicOzoneFileSystem.isDirectory/isFile now use the head-op path instead
  of the deprecated super.isDirectory/isFile (which triggered a full
  getFileStatus + pipeline refresh); BasicOzoneClientAdapterImpl forwards headOp
  to OzoneBucket.getFileStatus.
- DeleteKeyHandler type checks pass headOp=true.
- Trash-root type checks fetch the status once instead of exists() + two
  getFileStatus() calls for the same path.

Adds unit tests for headOp propagation through the o3fs adapter and for the
o3fs FileSystem isDirectory/isFile behaviour.

Co-authored-by: Cursor <cursoragent@cursor.com>
Change-Id: I2cf245c4731e662f0929ce30f731ce518b422813
Copilot AI review requested due to automatic review settings July 16, 2026 01:24

Copilot AI 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.

Pull request overview

This PR extends the existing “head-op” (type-only / metadata-only) getFileStatus optimization beyond ofs:// to o3fs:// and other remaining type-only callers, reducing unnecessary OM→SCM pipeline refresh work and eliminating redundant exists()/getFileStatus() RPC patterns where only entry type is needed.

Changes:

  • Add a headOp-aware getFileStatus override in BasicOzoneClientAdapterImpl and route BasicOzoneFileSystem.isDirectory/isFile through headOp=true status checks (instead of FileSystem.super.isDirectory/isFile).
  • Reduce redundant trash-root RPCs by fetching a single FileStatus and handling FileNotFoundException directly.
  • Update CLI FSO delete-path directory checks to request headOp=true, and add unit tests validating head-op propagation and exception behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java Uses headOp=true for isDirectory/isFile and optimizes trash-root checks to avoid redundant status calls.
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneClientAdapterImpl.java Implements getFileStatus(..., headOp) to forward to OzoneBucket.getFileStatus(key, headOp).
hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/BasicRootedOzoneClientAdapterImpl.java Optimizes trash-root lookup by replacing exists()+2x getFileStatus() with a single getFileStatus() and FNFE handling.
hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/DeleteKeyHandler.java Switches FSO directory-type checks to use bucket.getFileStatus(key, true) for type-only lookups.
hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestBasicOzoneClientAdapterHeadOp.java New unit tests ensuring the adapter threads headOp through and preserves exception mapping semantics.
hadoop-ozone/ozonefs-common/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystemHeadOp.java New unit tests ensuring BasicOzoneFileSystem.isDirectory/isFile uses headOp=true and preserves error behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jojochuang jojochuang marked this pull request as draft July 16, 2026 01:40
@jojochuang jojochuang marked this pull request as ready for review July 16, 2026 03:53
@jojochuang

Copy link
Copy Markdown
Contributor Author

@yandrey321

@yandrey321 yandrey321 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.

LGTM

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.

3 participants