Skip to content

Add resourceAllocation field to trace record#6973

Merged
pditommaso merged 21 commits intomasterfrom
add-resource-allocation-trace-field
Apr 7, 2026
Merged

Add resourceAllocation field to trace record#6973
pditommaso merged 21 commits intomasterfrom
add-resource-allocation-trace-field

Conversation

@pditommaso
Copy link
Copy Markdown
Member

Summary

  • Add resourceAllocation transient field to TraceRecord exposing scheduler-allocated resources (cpuShares, memoryMiB, accelerators, time)
  • Source the value from the last TaskAttempt.resources, falling back to TaskState.resourceAllocation when no attempts exist
  • Forward the field through TowerClient to the platform
  • Bump sched-client to 0.46.0-SNAPSHOT (renames TaskState.resourceRequirementresourceAllocation)

Test plan

  • Unit tests for getResourceAllocation() in SeqeraTaskHandlerTest (null state, single attempt, multiple attempts, fallback scenarios)
  • Unit test for resourceAllocation propagation in TowerClientTest.makeTasksReq()
  • Existing trace metadata test updated to verify resourceAllocation included in trace record

🤖 Generated with Claude Code

Expose scheduler-allocated resources (cpuShares, memoryMiB, accelerators,
time) in the trace record. The value is taken from the last TaskAttempt's
resources, falling back to the TaskState's resourceAllocation if no
attempts exist.

Also bump sched-client to 0.46.0-SNAPSHOT which renames
TaskState.resourceRequirement to resourceAllocation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@netlify
Copy link
Copy Markdown

netlify bot commented Mar 25, 2026

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit f5f840c
🔍 Latest deploy log https://app.netlify.com/projects/nextflow-docs-staging/deploys/69d53c449fb06c000847b643

This reverts commit 00f35b3.

The accelerator and accelerator_type fields in the trace record are
superseded by the resourceAllocation field which carries the actual
scheduler-allocated resources including accelerator info.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Copy link
Copy Markdown
Member Author

@bentsherman have a look at this. I think we can remove accelerator for "classic" trace and only keep as platform trace via resourceAllocation

@pditommaso pditommaso requested a review from bentsherman March 30, 2026 14:55
@pditommaso pditommaso marked this pull request as ready for review March 30, 2026 14:55
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
transient private ContainerMeta containerMeta
transient private Integer numSpotInterruptions
transient private String logStreamId
transient private Map<String,Object> resourceAllocation
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not blocking, but the naming is a bit awkward. how about resourcesAllocated ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Eventually, it's prefer to keep aligned to sched naming for now

pditommaso and others added 17 commits April 7, 2026 19:16
* Bump sched-client to 0.47.0 and update prediction model support

- Upgrade sched-client from 0.41.0-SNAPSHOT to 0.47.0
- Add qr/v2 prediction model to supported values description
- Remove client-side prediction model validation (moved to backend)
- Fix getResourceRequirement() -> getResourceAllocation() API change

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

* Fix test to use renamed resourceAllocation API

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

---------

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…i skip]

* Docs: document Wave support for module resources directory

Update Wave and module docs to explain that the module `resources/`
directory is automatically included in Wave-provisioned containers,
removing the need for ADD/COPY Dockerfile commands.

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

* Update docs/module.md [ci skip]

Co-authored-by: Chris Hakkaart <chris.hakkaart@seqera.io>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

* Update docs/wave.md [ci skip]

Co-authored-by: Chris Hakkaart <chris.hakkaart@seqera.io>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

---------

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Chris Hakkaart <chris.hakkaart@seqera.io>
---------

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Co-authored-by: Ben Sherman <bentshermann@gmail.com>
Co-authored-by: Ben Sherman <bentshermann@gmail.com>
Co-authored-by: Ben Sherman <bentshermann@gmail.com>
Co-authored-by: Ben Sherman <bentshermann@gmail.com>
Co-authored-by: Ben Sherman <bentshermann@gmail.com>
Co-authored-by: Ben Sherman <bentshermann@gmail.com>
… fast]

* Use npr-client API instead of custom ModuleRegistryClient

Replace the custom ModuleRegistryClient with the npr-client library,
delegating HTTP registry interactions to the shared client. This removes
~500 lines of duplicated HTTP/retry/auth logic.

- Delete ModuleRegistryClient.groovy
- Update call sites to use new npr-client method names
  (getModule, getModuleRelease, searchModules, downloadModuleRelease,
   publishModuleRelease)
- Update all tests to match new API

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

* Bump npr-api and npr-client to 0.22.0 [ci fast]

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>

---------

Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Keep the existing trace fields for requested accelerators alongside
the new resourceAllocation field for allocated resources.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Set accelerator request count and type in the trace record from
the process accelerator directive. Add corresponding test coverage.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso pditommaso requested review from a team as code owners April 7, 2026 17:17
@pditommaso pditommaso requested a review from bentsherman April 7, 2026 17:17
@pditommaso pditommaso merged commit a274293 into master Apr 7, 2026
25 of 26 checks passed
@pditommaso pditommaso deleted the add-resource-allocation-trace-field branch April 7, 2026 19:50
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.

6 participants