feat(computing-unit-managing-service): add admin endpoint to list all computing units#6854
Draft
kunwp1 wants to merge 3 commits into
Draft
feat(computing-unit-managing-service): add admin endpoint to list all computing units#6854kunwp1 wants to merge 3 commits into
kunwp1 wants to merge 3 commits into
Conversation
…nd batch k8s calls Centralize the computing-unit listing logic in ComputingUnitHelpers so the per-user listing endpoint routes through a single, reusable implementation (also reused by a follow-up admin endpoint): - Add resolveOwnerInfo, partitionLiveUnits, reconcileVanishedKubernetesUnits, buildDashboardUnit, and the isKubernetes / podPhasesFor / podMetricsFor guards. - Remove the duplicated private getComputingUnitStatus / getComputingUnitMetrics from ComputingUnitManagingResource; callers use the shared helpers. - Add bulk KubernetesClient.getAllPodPhases / getAllPodMetrics (one namespace-wide list()/top() each) plus a shared containerUsage helper and a fetchPodMetricsItems extraction; listComputingUnits now resolves pod status/metrics in bulk instead of probing podExists twice per unit and re-fetching top() per unit. - Add a package-private KubernetesClient test seam (setClientForTesting) so the fabric8 client can be stubbed in tests; not used in production. API response behavior is unchanged; per-user reconciliation now runs over cuid-deduplicated units, reducing redundant DB updates. Tests: ComputingUnitHelpersSpec (embedded Postgres via MockTexeraDB), KubernetesClientSpec (stubbed fabric8 client), and a new ComputingUnitManagingResourceSpec driving the per-user endpoints over the embedded DB. build.sbt gains DAO/Auth test artifacts and serial test execution. Generated-by: Claude Code, Claude Opus 4.8
… computing units Add an ADMIN-only GET /computing-unit/admin/list that returns every non-terminated computing unit across all users, backing the admin Computing Units dashboard. - AdminComputingUnitResource filters active units in SQL (excludes rows with a non-null terminate_time), reconciles vanished pods via the shared ComputingUnitHelpers, and assembles rows with each owner's name/avatar. - accessPrivilege is reported as WRITE for admin rows (an admin can manage any unit it can see); isOwner reflects whether the requesting admin owns the unit. - Register the resource in ComputingUnitManagingService and extend the startup access-control coverage check to include it. Builds on the shared listing helpers introduced in the preceding refactor. Tests: AdminComputingUnitResourceSpec drives listAllComputingUnits end-to-end over the embedded DB (terminated rows excluded, WRITE access, isOwner); ComputingUnitManagingServiceRunSpec boots run() and verifies the admin resource registers (DB-gated: connects only when Postgres is reachable, cancels otherwise). Generated-by: Claude Code, Claude Opus 4.8
github-actions
Bot
added
feature
dependencies
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6854 +/- ##
============================================
+ Coverage 69.77% 74.94% +5.17%
+ Complexity 3379 2339 -1040
============================================
Files 1142 877 -265
Lines 44828 36666 -8162
Branches 4948 3823 -1125
============================================
- Hits 31277 27478 -3799
+ Misses 11895 7934 -3961
+ Partials 1656 1254 -402
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 424 | 0.259 | 23,074/30,020/30,020 us | 🔴 +14.5% / 🔴 +87.5% |
| ⚪ | bs=100 sw=10 sl=64 | 961 | 0.586 | 101,100/129,419/129,419 us | ⚪ within ±5% / 🔴 +21.6% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,091 | 0.666 | 917,520/945,057/945,057 us | ⚪ within ±5% / 🟢 -10.0% |
Baseline details
Latest main 17cdba4 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 424 tuples/sec | 491 tuples/sec | 767.66 tuples/sec | -13.6% | -44.8% |
| bs=10 sw=10 sl=64 | MB/s | 0.259 MB/s | 0.3 MB/s | 0.469 MB/s | -13.7% | -44.7% |
| bs=10 sw=10 sl=64 | p50 | 23,074 us | 20,156 us | 12,623 us | +14.5% | +82.8% |
| bs=10 sw=10 sl=64 | p95 | 30,020 us | 29,926 us | 16,011 us | +0.3% | +87.5% |
| bs=10 sw=10 sl=64 | p99 | 30,020 us | 29,926 us | 18,856 us | +0.3% | +59.2% |
| bs=100 sw=10 sl=64 | throughput | 961 tuples/sec | 977 tuples/sec | 998.44 tuples/sec | -1.6% | -3.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.586 MB/s | 0.596 MB/s | 0.609 MB/s | -1.7% | -3.8% |
| bs=100 sw=10 sl=64 | p50 | 101,100 us | 100,927 us | 100,463 us | +0.2% | +0.6% |
| bs=100 sw=10 sl=64 | p95 | 129,419 us | 127,070 us | 106,400 us | +1.8% | +21.6% |
| bs=100 sw=10 sl=64 | p99 | 129,419 us | 127,070 us | 117,712 us | +1.8% | +9.9% |
| bs=1000 sw=10 sl=64 | throughput | 1,091 tuples/sec | 1,091 tuples/sec | 1,034 tuples/sec | 0.0% | +5.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.666 MB/s | 0.666 MB/s | 0.631 MB/s | 0.0% | +5.5% |
| bs=1000 sw=10 sl=64 | p50 | 917,520 us | 915,558 us | 973,294 us | +0.2% | -5.7% |
| bs=1000 sw=10 sl=64 | p95 | 945,057 us | 974,383 us | 1,019,213 us | -3.0% | -7.3% |
| bs=1000 sw=10 sl=64 | p99 | 945,057 us | 974,383 us | 1,049,896 us | -3.0% | -10.0% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,472.22,200,128000,424,0.259,23074.37,30019.57,30019.57
1,100,10,64,20,2082.16,2000,1280000,961,0.586,101099.89,129419.36,129419.36
2,1000,10,64,20,18328.86,20000,12800000,1091,0.666,917520.39,945056.84,945056.84…cle hooks Mirror the apache#6853 review fix onto the stacked admin PR so the whole contribution is uniform: beforeAll/afterAll overrides call super.beforeAll()/super.afterAll() (teardown via try/finally) in the shared specs and in AdminComputingUnitResourceSpec. No behavior change. Generated-by: Claude Code, Claude Opus 4.8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Adds an ADMIN-only endpoint that returns every non-terminated computing unit across all users. Needed for the admin Computing Units dashboard.
The endpoint queries all units via
WorkflowComputingUnitDao, excludes rows with a non-nullterminate_time, joinsUserDaofor each owner's name and avatar, and reuses the sharedComputingUnitHelpers(introduced in the preceding refactor PR) to reconcile vanished pods and resolve status/metrics.The new resource is registered in
ComputingUnitManagingService, and the startup access-control coverage check is extended to include it.accessPrivilegeis reported asWRITEfor admin rows (an admin can manage any unit it can see), andisOwnerreflects whether the requesting admin happens to own the unit.Any related issues, documentation, discussions?
Closes #6477. Part of #6476 (Admin Computing Units Dashboard). Originates from discussion #6322.
How was this PR tested?
Added
AdminComputingUnitResourceSpec(driveslistAllComputingUnitsend-to-end over the embedded DB: terminated rows excluded,WRITEaccess,isOwner) and extendedComputingUnitManagingServiceRunSpecto verify the admin resource registers.Ran
sbt 'ComputingUnitManagingService/testOnly org.apache.texera.service.resource.AdminComputingUnitResourceSpec org.apache.texera.service.ComputingUnitManagingServiceRunSpec'(8 passed, 1 canceled — the DB-gatedrun()registration test cancels when no Postgres is reachable) andsbt 'ComputingUnitManagingService/scalafmtCheck; ComputingUnitManagingService/Test/scalafmtCheck'(clean).Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code, Claude Opus 4.8