refactor(config): drop amber prefix from common config package#5672
Conversation
…late in issues or prs
Ci/template compliance warning
Yicong-Huang
left a comment
There was a problem hiding this comment.
Thanks! Please see inline comments. We still want to prevent importing web service related packages from amber. for common package, it is an exception place to put packages needed both on web and engine (amber) side.
to org.apache.texera.common.config
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5672 +/- ##
============================================
- Coverage 52.85% 52.64% -0.21%
- Complexity 2620 2621 +1
============================================
Files 1090 1085 -5
Lines 42176 41850 -326
Branches 4531 4481 -50
============================================
- Hits 22293 22033 -260
+ Misses 18573 18535 -38
+ Partials 1310 1282 -28
*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 | 462 | 0.282 | 21,569/26,121/26,121 us | 🟢 -28.8% / 🟢 -25.3% |
| 🔴 | bs=100 sw=10 sl=64 | 940 | 0.574 | 105,715/121,910/121,910 us | 🔴 +5.9% / 🟢 -12.8% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,139 | 0.695 | 877,821/916,912/916,912 us | ⚪ within ±5% / 🟢 -10.4% |
Baseline details
Latest main aca41f3 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 462 tuples/sec | 441 tuples/sec | 410.82 tuples/sec | +4.8% | +12.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.282 MB/s | 0.269 MB/s | 0.251 MB/s | +4.8% | +12.5% |
| bs=10 sw=10 sl=64 | p50 | 21,569 us | 21,788 us | 23,785 us | -1.0% | -9.3% |
| bs=10 sw=10 sl=64 | p95 | 26,121 us | 36,679 us | 34,980 us | -28.8% | -25.3% |
| bs=10 sw=10 sl=64 | p99 | 26,121 us | 36,679 us | 34,980 us | -28.8% | -25.3% |
| bs=100 sw=10 sl=64 | throughput | 940 tuples/sec | 986 tuples/sec | 891.94 tuples/sec | -4.7% | +5.4% |
| bs=100 sw=10 sl=64 | MB/s | 0.574 MB/s | 0.602 MB/s | 0.544 MB/s | -4.7% | +5.4% |
| bs=100 sw=10 sl=64 | p50 | 105,715 us | 99,787 us | 112,277 us | +5.9% | -5.8% |
| bs=100 sw=10 sl=64 | p95 | 121,910 us | 118,562 us | 139,802 us | +2.8% | -12.8% |
| bs=100 sw=10 sl=64 | p99 | 121,910 us | 118,562 us | 139,802 us | +2.8% | -12.8% |
| bs=1000 sw=10 sl=64 | throughput | 1,139 tuples/sec | 1,122 tuples/sec | 1,041 tuples/sec | +1.5% | +9.4% |
| bs=1000 sw=10 sl=64 | MB/s | 0.695 MB/s | 0.685 MB/s | 0.635 MB/s | +1.5% | +9.4% |
| bs=1000 sw=10 sl=64 | p50 | 877,821 us | 893,930 us | 972,714 us | -1.8% | -9.8% |
| bs=1000 sw=10 sl=64 | p95 | 916,912 us | 952,111 us | 1,023,057 us | -3.7% | -10.4% |
| bs=1000 sw=10 sl=64 | p99 | 916,912 us | 952,111 us | 1,023,057 us | -3.7% | -10.4% |
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,433.08,200,128000,462,0.282,21568.96,26121.21,26121.21
1,100,10,64,20,2126.94,2000,1280000,940,0.574,105714.97,121909.83,121909.83
2,1000,10,64,20,17552.29,20000,12800000,1139,0.695,877821.17,916912.33,916912.33
What changes were proposed in this PR?
common/configunder one package,org.apache.texera.common.config. This merges the two former packages (org.apache.texera.configandorg.apache.texera.amber.config) so the namespace reflects that they live incommonand are shared by both web and engine, rather than implying they belong toamber..../org/apache/texera/common/config/and rewrote all references across the repo (74 files) to the new package.Any related issues, documentation, discussions?
Closes: #5668
How was this PR tested?
sbt "Config/compile" "ConfigService/compile" "ComputingUnitManagingService/compile" "WorkflowCompilingService/compile" "WorkflowCore/compile"all report[success].Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.8 in compliance with ASF