Skip to content

Google Cloud SDK Shading for GCS PerfIO#15130

Open
sdrp713 wants to merge 2 commits into
NVIDIA:mainfrom
sdrp713:gcs-shading
Open

Google Cloud SDK Shading for GCS PerfIO#15130
sdrp713 wants to merge 2 commits into
NVIDIA:mainfrom
sdrp713:gcs-shading

Conversation

@sdrp713

@sdrp713 sdrp713 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Description

Relocate the bundled Google client stack in the aggregator shade configuration to isolate the GCS SDK from Dataproc-provided Google/GCS connector libraries.

  • Relocate com.google, io.grpc, and io.opencensus in both Scala 2.12 and Scala 2.13 aggregator POMs.
  • Preserve the existing FlatBuffers relocation behavior through the broader com.google relocation.

Enable GCS PerfIO automatically by using the resolved executor-side PerfIO state instead of requiring spark.rapids.perfio.gcs.enabled=true to be explicitly set in SparkConf.

  • Update RapidsInputFiles.isGCSPerfEnabled() to call PerfIO.isGCSPerfEnabled().
  • Preserve explicit disable behavior through private PerfIO initialization.

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
    (Please provide the names of the existing tests in the PR description.)
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Signed-off-by: Rahul Prabhu <raprabhu@nvidia.com>
@sdrp713 sdrp713 self-assigned this Jun 23, 2026
@sdrp713 sdrp713 requested a review from a team as a code owner June 23, 2026 22:44
@greptile-apps

greptile-apps Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR does two things: expands the Maven shade configuration in both aggregator POMs to relocate the full Google client library stack (com.google, io.grpc, io.opencensus) instead of only com.google.flatbuffers, isolating the plugin's bundled GCS SDK from Dataproc's cluster-provided Google libraries; and changes isGCSPerfEnabled() to delegate to PerfIO.isGCSPerfEnabled() so GCS PerfIO is auto-enabled once the executor initializes PerfIO instead of requiring an explicit spark.rapids.perfio.gcs.enabled=true config key.

  • aggregator/pom.xml and scala2.13/aggregator/pom.xml: The com.google.flatbuffers-only relocation is replaced with a broad com.google relocation (excluding com.google.protobuf to preserve ORC/Spark interop) plus new io.grpc and io.opencensus relocations. Both Scala 2.12 and 2.13 aggregators are updated identically.
  • RapidsInputFiles.java: isGCSPerfEnabled() now calls PerfIO.isGCSPerfEnabled() rather than reading PerfIOConf.GCSPERF_ENABLED() from SparkConf; the existing SparkEnv null-guard is preserved as an executor-initialization check.

Confidence Score: 5/5

Safe to merge. The shade configuration change is well-scoped and correctly documented, and the GCS PerfIO auto-enable logic cleanly delegates to executor-side state with the existing SparkEnv null guard preserved.

All three files contain straightforward, well-commented changes. The com.google relocation broadens an existing pattern to cover the full GCS SDK stack; the protobuf exclusion correctly preserves cross-boundary interop with Spark and ORC. The Guava usages present in the plugin (Objects.hashCode, @VisibleForTesting, ThreadFactoryBuilder) are purely internal and do not cross the shade boundary, so no ClassCastException risk was found. The isGCSPerfEnabled change is a clean delegation with no missing null-safety. The only nit is an unused local variable that can be removed without changing behavior.

No files require special attention.

Important Files Changed

Filename Overview
aggregator/pom.xml Replaces narrow com.google.flatbuffers relocation with broad com.google relocation (excluding com.google.protobuf) and adds io.grpc and io.opencensus relocations to isolate the GCS SDK. Change is well-documented and protobuf exclusion correctly preserves Spark/ORC interop.
scala2.13/aggregator/pom.xml Identical shade configuration change as aggregator/pom.xml for Scala 2.13. Both POMs are kept in sync as expected.
sql-plugin/src/main/java/com/nvidia/spark/rapids/fileio/RapidsInputFiles.java isGCSPerfEnabled() changed to delegate to PerfIO.isGCSPerfEnabled() instead of reading SparkConf directly. The SparkEnv null-guard is preserved but the retrieved env variable is only used for the null check and not subsequently.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant HFI as HadoopFileIO
    participant RIF as RapidsInputFiles
    participant PIO as PerfIO (executor state)
    participant GCS as GCSInputFile

    HFI->>RIF: isGCSPerfEnabled()
    RIF->>RIF: SparkEnv.get() null check
    alt SparkEnv is null
        RIF-->>HFI: false (pre-init path)
    else SparkEnv live
        RIF->>PIO: isGCSPerfEnabled()
        PIO-->>RIF: true/false (executor-resolved state)
        RIF-->>HFI: result
    end
    alt GCS PerfIO enabled
        HFI->>GCS: GCSInputFile.create(path, conf)
    else fallback
        HFI->>HFI: HadoopInputFile.create(path, conf)
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant HFI as HadoopFileIO
    participant RIF as RapidsInputFiles
    participant PIO as PerfIO (executor state)
    participant GCS as GCSInputFile

    HFI->>RIF: isGCSPerfEnabled()
    RIF->>RIF: SparkEnv.get() null check
    alt SparkEnv is null
        RIF-->>HFI: false (pre-init path)
    else SparkEnv live
        RIF->>PIO: isGCSPerfEnabled()
        PIO-->>RIF: true/false (executor-resolved state)
        RIF-->>HFI: result
    end
    alt GCS PerfIO enabled
        HFI->>GCS: GCSInputFile.create(path, conf)
    else fallback
        HFI->>HFI: HadoopInputFile.create(path, conf)
    end
Loading

Reviews (2): Last reviewed commit: "Exclude protobuf from relocation" | Re-trigger Greptile

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 updates the shaded “aggregator” build to better isolate Google/GCS client libraries from Dataproc-provided dependencies, and adjusts GCS PerfIO enablement so it follows executor-side PerfIO initialization state rather than requiring an explicit SparkConf flag.

Changes:

  • Expand aggregator shading relocations to relocate com.google, io.grpc, and io.opencensus (Scala 2.12 + 2.13).
  • Keep existing FlatBuffers relocation behavior implicitly via the broader com.google relocation.
  • Update RapidsInputFiles.isGCSPerfEnabled() to delegate to PerfIO.isGCSPerfEnabled().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
sql-plugin/src/main/java/com/nvidia/spark/rapids/fileio/RapidsInputFiles.java Switches GCS PerfIO gating from a SparkConf boolean to PerfIO’s resolved executor-side state.
aggregator/pom.xml Updates shading relocations to isolate bundled Google client stack from Dataproc classpath.
scala2.13/aggregator/pom.xml Same shading relocation changes for the Scala 2.13 aggregator build.

Comment thread aggregator/pom.xml
Comment thread scala2.13/aggregator/pom.xml
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.

4 participants