Skip to content

fix(FR-2837): set shmem under config.resource_opts and display SHM in endpoint detail#7299

Merged
graphite-app[bot] merged 1 commit into
mainfrom
fix/FR-2837-shmem-resource-opts
May 12, 2026
Merged

fix(FR-2837): set shmem under config.resource_opts and display SHM in endpoint detail#7299
graphite-app[bot] merged 1 commit into
mainfrom
fix/FR-2837-shmem-resource-opts

Conversation

@agatha197
Copy link
Copy Markdown
Contributor

@agatha197 agatha197 commented May 8, 2026

Resolves #7292 (FR-2837)

Test server: <webui>/serving/dc384809-56d5-4a17-9d2e-73fde29af978 in 10.82.1.246

Summary

  • Move shmem from being set directly under config to under config.resource_opts.shmem when creating model services in ServiceLauncherPageContent, matching the pattern used by useStartSession for compute sessions.
  • Display SHM next to the memory resource on the endpoint detail page by parsing endpoint.resource_opts.shmem and converting it to bytes via convertToBinaryUnit, so users can verify the configured shared memory size of a deployed model service.

Test plan

  • Create a new model service with custom shared memory (e.g. 2 GiB) and verify the deployment is created with the requested SHM (not 0.06 GiB).
  • Open the endpoint detail page for the created service and confirm (SHM: …GiB) appears next to the memory value.
  • Verify that when shmem is not set, the memory display does not show an empty/zero SHM hint.

Copy link
Copy Markdown
Contributor Author

agatha197 commented May 8, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions github-actions Bot added the size:S 10~30 LoC label May 8, 2026
@agatha197 agatha197 marked this pull request as ready for review May 8, 2026 02:36
@agatha197 agatha197 requested review from Copilot, ironAiken2, nowgnuesLee and yomybaby and removed request for Copilot May 8, 2026 02:36
Copy link
Copy Markdown
Contributor

@ironAiken2 ironAiken2 left a comment

Choose a reason for hiding this comment

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

The changes in this PR appear to target the legacy /serving code paths, not the new /deployments UI that is actually wired up:

  • ServiceLauncherPageContent is only rendered by pages/ServiceLauncherPage.tsx, and that page is no longer imported in routes.tsx (see the comment around routes.tsx:70-76 referencing FR-2675 / FR-2822). The legacy /serving and /service routes are now just WebUINavigate redirects to /deployments. Other importers of ServiceLauncherPageContent (useModelServiceLauncher, LegacyModelTryContentButton, ServiceValidationView) only import its types, not the component itself, so the submit handler that was patched here is never actually executed in the running app.
  • EndpointDetailPage is not imported anywhere in routes.tsx either. The detail route under /deployments/:deploymentId (and /admin-deployments/:deploymentId) renders DeploymentDetailPage, not EndpointDetailPage. So the new SHM display block also never reaches the UI.

As a result, neither half of the fix is observable from the user-facing flow:

  1. Submit side — when a revision is created with a custom shmem in the new deployment UI, the shmem still needs to be placed under config.resource_opts.shmem by the live code path. That path lives in DeploymentLauncherPageContent.tsx (initial deployment) and DeploymentAddRevisionModal.tsx (revision creation — currently builds a resourceConfig.resourceOpts.entries[] shape around line 668), not in ServiceLauncherPageContent.
  2. Display side — opening the revision detail drawer on a deployment, I cannot see the configured shmem anywhere. The SHM hint added to EndpointDetailPage does not appear because that page is not rendered. The display needs to be added to the DeploymentDetailPage (or the revision detail drawer component that is actually used).

Could you reapply the fix against the deployment-side components (DeploymentLauncherPageContent, DeploymentAddRevisionModal, and DeploymentDetailPage / revision detail drawer) and confirm the test by creating a deployment + revision with a custom shmem and checking that the value is both sent correctly and shown in the drawer? Also worth verifying the testing notes in the PR description — /serving/<id> is a redirect to /deployments/<id>, so the test server screenshot would have been rendered by DeploymentDetailPage, which this PR does not touch.

If ServiceLauncherPage / EndpointDetailPage are now genuinely unreachable, a follow-up cleanup PR to delete them (or roll them into this one) would also help avoid this kind of confusion in the future.

@agatha197 agatha197 force-pushed the fix/FR-2837-shmem-resource-opts branch from 059427c to cf22153 Compare May 11, 2026 16:05
Copilot AI review requested due to automatic review settings May 11, 2026 16:05
@agatha197 agatha197 changed the base branch from main to 25.15 May 11, 2026 16:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@agatha197 agatha197 changed the base branch from 25.15 to graphite-base/7299 May 11, 2026 16:48
@agatha197 agatha197 force-pushed the graphite-base/7299 branch from 78fb36a to 7a4501f Compare May 11, 2026 16:48
@agatha197 agatha197 changed the base branch from graphite-base/7299 to main May 11, 2026 16:48
Copy link
Copy Markdown
Contributor

@ironAiken2 ironAiken2 left a comment

Choose a reason for hiding this comment

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

LGTM - This is a pull request for 25.15 Cherrypick.

@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented May 12, 2026

Merge activity

… endpoint detail (#7299)

Resolves #7292 ([FR-2837](https://lablup.atlassian.net/browse/FR-2837))

> Test server: \<webui>/serving/dc384809-56d5-4a17-9d2e-73fde29af978 in `10.82.1.246`

## Summary

- Move `shmem` from being set directly under `config` to under `config.resource_opts.shmem` when creating model services in `ServiceLauncherPageContent`, matching the pattern used by `useStartSession` for compute sessions.
- Display SHM next to the memory resource on the endpoint detail page by parsing `endpoint.resource_opts.shmem` and converting it to bytes via `convertToBinaryUnit`, so users can verify the configured shared memory size of a deployed model service.

## Test plan

- [ ] Create a new model service with custom shared memory (e.g. 2 GiB) and verify the deployment is created with the requested SHM (not 0.06 GiB).
- [ ] Open the endpoint detail page for the created service and confirm `(SHM: …GiB)` appears next to the memory value.
- [ ] Verify that when `shmem` is not set, the memory display does not show an empty/zero SHM hint.

[FR-2837]: https://lablup.atlassian.net/browse/FR-2837?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@graphite-app graphite-app Bot force-pushed the fix/FR-2837-shmem-resource-opts branch from cf22153 to b3fa810 Compare May 12, 2026 04:47
@graphite-app graphite-app Bot merged commit b3fa810 into main May 12, 2026
11 checks passed
@graphite-app graphite-app Bot deleted the fix/FR-2837-shmem-resource-opts branch May 12, 2026 04:48
@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for react-coverage (./react)

Status Category Percentage Covered / Total
🔵 Lines 6.46% 1783 / 27578
🔵 Statements 5.31% 1978 / 37191
🔵 Functions 5.18% 296 / 5706
🔵 Branches 3.72% 1293 / 34729
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
react/src/components/ServiceLauncherPageContent.tsx 0% 0% 0% 0% 121-2200
react/src/pages/EndpointDetailPage.tsx 0% 0% 0% 0% 130-1611
Generated in workflow #545 for commit b3fa810 by the Vitest Coverage Report Action

yomybaby added a commit that referenced this pull request May 12, 2026
… endpoint detail (#7299)

Cherry-picked from b3fa810.
Resolves #7292 (FR-2837).

- Move shmem from being set directly under config to under config.resource_opts.shmem
  when creating model services in ServiceLauncherPageContent.
- Display SHM next to the memory resource on the endpoint detail page by parsing
  endpoint.resource_opts.shmem and converting it to bytes via convertToBinaryUnit.

Conflict resolution notes (25.15 branch):
- ServiceLauncherPageContent: 25.15 already had `resource_opts: { shmem }` unconditionally;
  adopted PR's conditional guard `...(values.resource.shmem && { resource_opts: { shmem } })`.
- EndpointDetailPage imports: only added `convertToBinaryUnit` (the other extra exports
  from main are not used in 25.15's version of this file).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10~30 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shared memory fixed at 0.06GiB when creating model services in version 26.4.3

3 participants