refactor(amber): make the hub and unified search resource-agnostic - #7928
Conversation
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7928 +/- ##
============================================
+ Coverage 93.22% 93.25% +0.03%
+ Complexity 4666 4661 -5
============================================
Files 1175 1176 +1
Lines 47645 47566 -79
Branches 5319 5300 -19
============================================
- Hits 44417 44358 -59
+ Misses 1749 1735 -14
+ Partials 1479 1473 -6
*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 | 563 | 0.344 | 17,051/20,933/20,933 us | 🟢 -49.4% / 🔴 +56.3% |
| 🟢 | bs=100 sw=10 sl=64 | 1,222 | 0.746 | 82,837/94,687/94,687 us | 🟢 -18.4% / ⚪ within ±5% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,404 | 0.857 | 713,163/764,485/764,485 us | ⚪ within ±5% / 🟢 -16.5% |
Baseline details
Latest main 5de9bf9 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 563 tuples/sec | 501 tuples/sec | 908.02 tuples/sec | +12.4% | -38.0% |
| bs=10 sw=10 sl=64 | MB/s | 0.344 MB/s | 0.306 MB/s | 0.554 MB/s | +12.4% | -37.9% |
| bs=10 sw=10 sl=64 | p50 | 17,051 us | 14,730 us | 10,907 us | +15.8% | +56.3% |
| bs=10 sw=10 sl=64 | p95 | 20,933 us | 41,331 us | 13,604 us | -49.4% | +53.9% |
| bs=10 sw=10 sl=64 | p99 | 20,933 us | 41,331 us | 16,697 us | -49.4% | +25.4% |
| bs=100 sw=10 sl=64 | throughput | 1,222 tuples/sec | 1,250 tuples/sec | 1,188 tuples/sec | -2.2% | +2.9% |
| bs=100 sw=10 sl=64 | MB/s | 0.746 MB/s | 0.763 MB/s | 0.725 MB/s | -2.2% | +2.9% |
| bs=100 sw=10 sl=64 | p50 | 82,837 us | 78,895 us | 85,003 us | +5.0% | -2.5% |
| bs=100 sw=10 sl=64 | p95 | 94,687 us | 116,081 us | 91,599 us | -18.4% | +3.4% |
| bs=100 sw=10 sl=64 | p99 | 94,687 us | 116,081 us | 99,202 us | -18.4% | -4.6% |
| bs=1000 sw=10 sl=64 | throughput | 1,404 tuples/sec | 1,435 tuples/sec | 1,221 tuples/sec | -2.2% | +15.0% |
| bs=1000 sw=10 sl=64 | MB/s | 0.857 MB/s | 0.876 MB/s | 0.745 MB/s | -2.2% | +15.0% |
| bs=1000 sw=10 sl=64 | p50 | 713,163 us | 693,725 us | 837,593 us | +2.8% | -14.9% |
| bs=1000 sw=10 sl=64 | p95 | 764,485 us | 762,229 us | 881,015 us | +0.3% | -13.2% |
| bs=1000 sw=10 sl=64 | p99 | 764,485 us | 762,229 us | 915,550 us | +0.3% | -16.5% |
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,355.32,200,128000,563,0.344,17050.72,20933.24,20933.24
1,100,10,64,20,1636.89,2000,1280000,1222,0.746,82836.56,94686.58,94686.58
2,1000,10,64,20,14243.99,20000,12800000,1404,0.857,713162.98,764485.31,764485.31…aliases The projection assertions added upstream in apache#7855 pin the pre-refactor alias names. The id, publicity, downloadable, privilege and cover slots are now shared with the other versioned resources, so their aliases are resource-neutral while the columns feeding them stay the dataset's. Also drops a stray indent in fetchDashboardVersionedResourcesByIds left by the merge.
25ebef8 to
2b40030
Compare
|
/request-review @aicam |
What changes were proposed in this PR?
The hub dispatched on
EntityTypein four separate places, andDatasetSearchQueryBuildercarried its own copy of the access-scoped join, the keyword/date/id filter and the
LakeFS-sized hydration. Adding a second resource type meant four dispatch edits plus a
clone of the query logic, and two of those sites compiled cleanly but failed at runtime.
EntityTablesnow exposes oneEntityTableSetper entity type, withcloneTableas anOptionand a newAccessTablethat removes the non-exhaustive match inHubResource.userAccess.VersionedResourceTablesnames the columns of a LakeFS-backed resource;VersionedResourceSearchQueryBuilderholds the single copy of FROM / WHERE / hydration.DatasetSearchQueryBuilderkeeps only its projection.UnifiedResourceSchemabecome shared versioned-resourceslots, so the next resource type widens the union by two columns instead of seven.
Two existing bugs are fixed as a result:
getTops?entityType=datasetwith noactionTypesno longer 500s on a throwing
CloneTable(Dataset), andgetCounts' clone guard now askswhether the entity type has a clone table instead of whether it is not a dataset.
No user-facing behavior changes.
Any related issues, documentation, discussions?
Prepares the amber side of #6501 (part of #6494): adding models becomes one descriptor plus
one registry entry. Contains #6872 and #7922, so their commits show in the diff until they
merge.
How was this PR tested?
Existing suites, unmodified except where they pin the renamed projection aliases:
WorkflowExecutionService/testOnly *dashboard*— 467/467, andFileService/test— 376/376.New
VersionedResourceTablesSpeccovers the descriptor contract and the access-joinpredicate;
EntityTablesSpeccovers the registry; regression tests added for the two bugsabove (
getTopswith noactionTypes, and the clone bucket for a cloneless entity type).Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)