refactor(asf): unify Dropwizard service bootstrap into common/auth#5983
refactor(asf): unify Dropwizard service bootstrap into common/auth#5983Ma77Ball wants to merge 2 commits into
Conversation
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (29.54%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #5983 +/- ##
============================================
+ Coverage 56.86% 56.96% +0.09%
+ Complexity 3027 3025 -2
============================================
Files 1127 1128 +1
Lines 43756 43693 -63
Branches 4736 4735 -1
============================================
+ Hits 24883 24888 +5
+ Misses 17401 17333 -68
Partials 1472 1472
*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:
|
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 347 | 0.212 | 26,765/47,270/47,270 us | 🔴 +44.5% / 🔴 +213.7% |
| ⚪ | bs=100 sw=10 sl=64 | 784 | 0.478 | 125,406/141,733/141,733 us | ⚪ within ±5% / 🔴 +31.7% |
| ⚪ | bs=1000 sw=10 sl=64 | 916 | 0.559 | 1,089,944/1,124,525/1,124,525 us | ⚪ within ±5% / 🔴 +10.4% |
Baseline details
Latest main 1073b22 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 347 tuples/sec | 417 tuples/sec | 777.62 tuples/sec | -16.8% | -55.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.212 MB/s | 0.255 MB/s | 0.475 MB/s | -16.9% | -55.3% |
| bs=10 sw=10 sl=64 | p50 | 26,765 us | 23,498 us | 12,612 us | +13.9% | +112.2% |
| bs=10 sw=10 sl=64 | p95 | 47,270 us | 32,711 us | 15,070 us | +44.5% | +213.7% |
| bs=10 sw=10 sl=64 | p99 | 47,270 us | 32,711 us | 18,360 us | +44.5% | +157.5% |
| bs=100 sw=10 sl=64 | throughput | 784 tuples/sec | 819 tuples/sec | 988.31 tuples/sec | -4.3% | -20.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.478 MB/s | 0.5 MB/s | 0.603 MB/s | -4.4% | -20.8% |
| bs=100 sw=10 sl=64 | p50 | 125,406 us | 121,122 us | 101,066 us | +3.5% | +24.1% |
| bs=100 sw=10 sl=64 | p95 | 141,733 us | 147,693 us | 107,594 us | -4.0% | +31.7% |
| bs=100 sw=10 sl=64 | p99 | 141,733 us | 147,693 us | 115,830 us | -4.0% | +22.4% |
| bs=1000 sw=10 sl=64 | throughput | 916 tuples/sec | 911 tuples/sec | 1,019 tuples/sec | +0.5% | -10.1% |
| bs=1000 sw=10 sl=64 | MB/s | 0.559 MB/s | 0.556 MB/s | 0.622 MB/s | +0.5% | -10.2% |
| bs=1000 sw=10 sl=64 | p50 | 1,089,944 us | 1,096,368 us | 986,982 us | -0.6% | +10.4% |
| bs=1000 sw=10 sl=64 | p95 | 1,124,525 us | 1,135,091 us | 1,028,491 us | -0.9% | +9.3% |
| bs=1000 sw=10 sl=64 | p99 | 1,124,525 us | 1,135,091 us | 1,058,493 us | -0.9% | +6.2% |
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,575.78,200,128000,347,0.212,26764.86,47270.30,47270.30
1,100,10,64,20,2551.51,2000,1280000,784,0.478,125405.76,141733.33,141733.33
2,1000,10,64,20,21830.33,20000,12800000,916,0.559,1089944.04,1124524.81,1124524.81
What changes were proposed in this PR?
ServiceBootstrapincommon/authwith three shared helpers:configure(YAML env-var substitution +DefaultScalaModuleregistration),initDatabase(open the SQL connection pool fromStorageConfig), andconfigFilePath(resolve$TEXERA_HOME/<service>/src/main/resources/<file>).ServiceBootstrap, removing the duplicatedinitialize()/initConnection/main()boilerplate.WorkflowCompilingServiceat the sharedRequestLoggingFilter.registerinstead of its hand-rolled inline servlet filter, andNotebookMigrationServiceat the sharedAuthFeatures.registerinstead of its own identicalregisterAuthFeatures.jackson-module-scalaas aprovideddependency ofcommon/authforServiceBootstrap'sDefaultScalaModule.Any related issues, documentation, discussions?
Closes: #5982
How was this PR tested?
ServiceBootstrapSpec: runsbt -J-Dnet.bytebuddy.experimental=true "Auth/testOnly *ServiceBootstrapSpec", expect it to verifyconfigurewraps the config source provider and registers the Scala module, and thatconfigFilePathreturns an absolute path ending in the conventional<service>/src/main/resources/<file>suffix.sbt "ConfigService/testOnly *RunSpec" "AccessControlService/testOnly *RunSpec" "WorkflowCompilingService/testOnly *RunSpec" "ComputingUnitManagingService/testOnly *RunSpec" "NotebookMigrationService/testOnly *RunSpec"plusAuthFeaturesSpec, confirming each service still registers the same resources and auth stack.sbt "Auth/compile" "ConfigService/compile" "AccessControlService/compile" "FileService/compile" "ComputingUnitManagingService/compile" "WorkflowCompilingService/compile" "NotebookMigrationService/compile".-J-Dnet.bytebuddy.experimental=trueandFileServiceRunSpeccould not be instrumented locally; all of these run normally on CI's JDK 21.Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.8 in compliance with ASF