Skip to content

refactor(resource): deduplicate HealthCheckResource across services#5979

Open
Ma77Ball wants to merge 5 commits into
apache:mainfrom
Ma77Ball:refactor/shared-healthcheck-resource
Open

refactor(resource): deduplicate HealthCheckResource across services#5979
Ma77Ball wants to merge 5 commits into
apache:mainfrom
Ma77Ball:refactor/shared-healthcheck-resource

Conversation

@Ma77Ball

Copy link
Copy Markdown
Contributor

What changes were proposed in this PR?

  • Added a single shared HealthCheckResource in the common/auth module under the existing package org.apache.texera.service.resource, which all six backend services already depend on.
  • Deleted the six byte-for-byte identical copies from access-control, config, file, computing-unit-managing, workflow-compiling, and notebook-migration services (~160 lines removed).
  • Kept the same fully-qualified class name, so every existing import, register(classOf[HealthCheckResource]) call, and test spec works unchanged with zero call-site edits.
  • amber's org.apache.texera.web.resource.HealthCheckResource is intentionally left out of scope: it uses the older javax.ws.rs namespace and lacks @PermitAll, so it is not safely mergeable.

Any related issues, documentation, discussions?

Closes: #5976

How was this PR tested?

  • Pure consolidation with no behavior change, so no new test was added; the existing service *RunSpec suites already assert classOf[HealthCheckResource] registration and continue to pass against the shared class.
  • Compiled all affected modules: sbt "Auth/compile" "ConfigService/compile" "AccessControlService/compile" "FileService/compile" "ComputingUnitManagingService/compile" "WorkflowCompilingService/compile" "NotebookMigrationService/compile", expect all to succeed (they did locally).
  • Optional manual check: start any service and curl localhost:<port>/api/healthcheck, expect {"status":"ok"}.

Was this PR authored or co-authored using generative AI tooling?

Co-authored with Claude Opus 4.8 in compliance with ASF

HealthCheckResource.scala is duplicated amongst multiple services this
pr creates a shared resource to avoid code duplication
@github-actions github-actions Bot added refactor Refactor the code common platform Non-amber Scala service paths labels Jun 28, 2026
@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

  • Contributors with relevant context: @Yicong-Huang, @zyratlo
    You can notify them by mentioning @Yicong-Huang, @zyratlo in a comment.

@codecov-commenter

codecov-commenter commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.47%. Comparing base (0e0ec11) to head (52436e2).
⚠️ Report is 21 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5979      +/-   ##
============================================
+ Coverage     56.23%   56.47%   +0.24%     
- Complexity     2987     2999      +12     
============================================
  Files          1120     1126       +6     
  Lines         43167    43699     +532     
  Branches       4658     4734      +76     
============================================
+ Hits          24274    24679     +405     
- Misses        17472    17563      +91     
- Partials       1421     1457      +36     
Flag Coverage Δ *Carryforward flag
access-control-service 70.00% <ø> (ø)
agent-service 44.59% <ø> (ø)
amber 57.73% <ø> (-0.05%) ⬇️ Carriedforward from 22bb64c
computing-unit-managing-service 0.00% <ø> (ø)
config-service 52.30% <ø> (+0.74%) ⬆️
file-service 62.81% <ø> (+3.79%) ⬆️
frontend 49.97% <ø> (+0.69%) ⬆️
notebook-migration-service 78.57% <ø> (ø)
pyamber 90.20% <ø> (ø)
python 90.76% <ø> (ø) Carriedforward from 22bb64c
workflow-compiling-service 55.14% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Yicong-Huang

Copy link
Copy Markdown
Contributor

ah @Ma77Ball I think health check does not belong to auth module. I understand you want to have a common place for that, but not in auth..

@Yicong-Huang Yicong-Huang 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.

if needed, you can add common/resource project to contain this part of code.

@github-actions

github-actions Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 5 worse · ⚪ 8 noise (<±5%) · 0 without baseline

Compared against main 0e0ec11 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

config throughput MB/s latency max Δ latest / 7d
🔴 bs=10 sw=10 sl=64 376 0.23 26,325/32,018/32,018 us 🔴 +12.5% / 🔴 +112.5%
🔴 bs=100 sw=10 sl=64 785 0.479 125,203/146,663/146,663 us 🔴 +7.2% / 🔴 +34.7%
bs=1000 sw=10 sl=64 907 0.554 1,102,489/1,139,262/1,139,262 us ⚪ within ±5% / 🔴 +10.0%
Baseline details

Latest main 0e0ec11 from same runner

config metric PR latest main 7d avg Δ latest Δ 7d
bs=10 sw=10 sl=64 throughput 376 tuples/sec 402 tuples/sec 770.82 tuples/sec -6.5% -51.2%
bs=10 sw=10 sl=64 MB/s 0.23 MB/s 0.246 MB/s 0.47 MB/s -6.5% -51.1%
bs=10 sw=10 sl=64 p50 26,325 us 23,400 us 12,723 us +12.5% +106.9%
bs=10 sw=10 sl=64 p95 32,018 us 34,083 us 15,070 us -6.1% +112.5%
bs=10 sw=10 sl=64 p99 32,018 us 34,083 us 18,429 us -6.1% +73.7%
bs=100 sw=10 sl=64 throughput 785 tuples/sec 822 tuples/sec 973.75 tuples/sec -4.5% -19.4%
bs=100 sw=10 sl=64 MB/s 0.479 MB/s 0.502 MB/s 0.594 MB/s -4.6% -19.4%
bs=100 sw=10 sl=64 p50 125,203 us 121,481 us 102,519 us +3.1% +22.1%
bs=100 sw=10 sl=64 p95 146,663 us 136,833 us 108,855 us +7.2% +34.7%
bs=100 sw=10 sl=64 p99 146,663 us 136,833 us 117,788 us +7.2% +24.5%
bs=1000 sw=10 sl=64 throughput 907 tuples/sec 920 tuples/sec 1,004 tuples/sec -1.4% -9.7%
bs=1000 sw=10 sl=64 MB/s 0.554 MB/s 0.562 MB/s 0.613 MB/s -1.4% -9.6%
bs=1000 sw=10 sl=64 p50 1,102,489 us 1,092,596 us 1,001,930 us +0.9% +10.0%
bs=1000 sw=10 sl=64 p95 1,139,262 us 1,123,368 us 1,042,923 us +1.4% +9.2%
bs=1000 sw=10 sl=64 p99 1,139,262 us 1,123,368 us 1,074,893 us +1.4% +6.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,531.83,200,128000,376,0.230,26325.30,32017.55,32017.55
1,100,10,64,20,2549.15,2000,1280000,785,0.479,125202.76,146663.09,146663.09
2,1000,10,64,20,22039.34,20000,12800000,907,0.554,1102489.17,1139262.21,1139262.21

@Ma77Ball

Ma77Ball commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Moved HealthCheckResource into a new common/resource module.

@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label Jun 28, 2026
@Ma77Ball Ma77Ball requested a review from Yicong-Huang June 28, 2026 20:09
@Ma77Ball

Copy link
Copy Markdown
Contributor Author

/request-review @Yicong-Huang

@Yicong-Huang

Copy link
Copy Markdown
Contributor

this has nothing to do with ASF right? refactor(asf) let's change the title

Comment thread build.sbt
@Ma77Ball Ma77Ball changed the title refactor(asf): deduplicate HealthCheckResource across services refactor(resource): deduplicate HealthCheckResource across services Jun 29, 2026
added HealthCheckResourceSpec to test that the health check resource is
working and reports to codecov to get coverage results for this on
github
@github-actions github-actions Bot added the ci changes related to CI label Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci changes related to CI common dependencies Pull requests that update a dependency file platform Non-amber Scala service paths refactor Refactor the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deduplicate identical HealthCheckResource across backend services

3 participants