Skip to content

Add application-scoped YARN resource usage API - #267

Open
gerashegalov wants to merge 5 commits into
NVIDIA:devfrom
gerashegalov:feature/yarn-resource-cost-python-api-signed
Open

Add application-scoped YARN resource usage API#267
gerashegalov wants to merge 5 commits into
NVIDIA:devfrom
gerashegalov:feature/yarn-resource-cost-python-api-signed

Conversation

@gerashegalov

Copy link
Copy Markdown
Collaborator

Add a typed, packageable Python API for application-scoped YARN resource accounting.

The API accepts injected boto clients, preserves calculator and completeness evidence, returns per-instance seconds plus vcore/memory seconds, and leaves pricing policy to the caller. The existing CLI remains compatible.

Validation:

  • python3 -m unittest discover -s yarn-resource-cost -p "test*.py" (39 tests)
  • wheel build and isolated import smoke test

Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previously reported exact-key event-log issue is fixed and no new actionable failures remain.

Summary

  • Accepts injected EMR and S3 clients and returns structured completeness, retryability, calculator, container, and resource-usage evidence.
  • Supports S3 prefixes, rolling Spark event logs, and exact-key single-file event logs.
  • Reconciles late node-registration metadata and distinguishes retryable missing evidence from permanent accounting conflicts.
  • Adds Python packaging, an installed console entry point, documentation, CI checks, and API regression tests.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Request[EmrApplicationUsageRequest]
    EMR[Injected EMR client]
    S3[Injected S3 client]
    Materialize[Materialize Spark and YARN logs]
    Discover[Discover application metadata]
    Parse[Parse YARN accounting evidence]
    Calculate[Calculate application resource usage]
    Result[YarnApplicationUsageResult]

    Request --> Materialize
    EMR --> Materialize
    S3 --> Materialize
    Materialize --> Discover
    Materialize --> Parse
    Discover --> Calculate
    Parse --> Calculate
    Calculate --> Result
Loading

Reviews (4) · Last reviewed commit: "Support single-file S3 event logs"

Comment thread yarn-resource-cost/yarn_job_cost_api.py Outdated
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
Comment thread yarn-resource-cost/yarn_job_cost_api.py
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>

@rishic3 rishic3 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks good to me barring a perf comment.

Comment on lines +208 to +210
with tempfile.TemporaryDirectory(prefix="yarn-resource-cost-") as directory:
root = Path(directory)
event_root = _materialize_event_log(s3_client, request.event_log_uri, root / "events")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is walking event logs serially and could be very slow. It would be nice to have an option to accept a pre-materialized set of event logs, which tools can pass in.

@gerashegalov gerashegalov Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@rishic3 The existing API already supports this path: event_log_uri may be either an S3 URI or a local filesystem path. When the caller supplies a pre-materialized event-log file or directory, _materialize_event_log returns that path directly and performs no S3 listing or download. Aether can therefore pass its locally materialized event logs through the current request field. I agree this is not sufficiently obvious from the field name and current documentation; I can make the local-path behavior explicit in the API docs/README.

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