fix(kubernetes): terminate idle computing units - #6046
Conversation
|
👋 Thanks for opening this pull request, @yrenat! It looks like the pull request description doesn't quite follow our template yet:
Filling out the template helps reviewers understand and triage your contribution faster. Please edit the description to complete it. This message will disappear automatically once the template is followed. You can find the template prompts by editing the description, or see CONTRIBUTING.md for the full contribution flow. |
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6046 +/- ##
=========================================
Coverage 91.02% 91.03%
- Complexity 4454 4459 +5
=========================================
Files 1174 1174
Lines 47146 47127 -19
Branches 5287 5283 -4
=========================================
- Hits 42916 42903 -13
+ Misses 2550 2549 -1
+ Partials 1680 1675 -5
*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 | 404 | 0.247 | 23,525/34,106/34,106 us | 🔴 +14.0% / 🔴 +113.4% |
| 🔴 | bs=100 sw=10 sl=64 | 928 | 0.567 | 106,521/131,965/131,965 us | 🔴 +6.4% / 🔴 +23.0% |
| 🔴 | bs=1000 sw=10 sl=64 | 1,074 | 0.656 | 932,106/1,094,010/1,094,010 us | 🔴 +8.9% / 🔴 +7.1% |
Baseline details
Latest main 60300e3 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 404 tuples/sec | 456 tuples/sec | 779.07 tuples/sec | -11.4% | -48.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.247 MB/s | 0.278 MB/s | 0.476 MB/s | -11.2% | -48.1% |
| bs=10 sw=10 sl=64 | p50 | 23,525 us | 20,636 us | 12,818 us | +14.0% | +83.5% |
| bs=10 sw=10 sl=64 | p95 | 34,106 us | 30,899 us | 15,986 us | +10.4% | +113.4% |
| bs=10 sw=10 sl=64 | p99 | 34,106 us | 30,899 us | 19,339 us | +10.4% | +76.4% |
| bs=100 sw=10 sl=64 | throughput | 928 tuples/sec | 962 tuples/sec | 1,011 tuples/sec | -3.5% | -8.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.567 MB/s | 0.587 MB/s | 0.617 MB/s | -3.4% | -8.1% |
| bs=100 sw=10 sl=64 | p50 | 106,521 us | 103,060 us | 100,965 us | +3.4% | +5.5% |
| bs=100 sw=10 sl=64 | p95 | 131,965 us | 123,995 us | 107,295 us | +6.4% | +23.0% |
| bs=100 sw=10 sl=64 | p99 | 131,965 us | 123,995 us | 115,531 us | +6.4% | +14.2% |
| bs=1000 sw=10 sl=64 | throughput | 1,074 tuples/sec | 1,057 tuples/sec | 1,049 tuples/sec | +1.6% | +2.4% |
| bs=1000 sw=10 sl=64 | MB/s | 0.656 MB/s | 0.645 MB/s | 0.64 MB/s | +1.7% | +2.5% |
| bs=1000 sw=10 sl=64 | p50 | 932,106 us | 947,408 us | 978,248 us | -1.6% | -4.7% |
| bs=1000 sw=10 sl=64 | p95 | 1,094,010 us | 1,004,359 us | 1,021,881 us | +8.9% | +7.1% |
| bs=1000 sw=10 sl=64 | p99 | 1,094,010 us | 1,004,359 us | 1,050,075 us | +8.9% | +4.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,494.52,200,128000,404,0.247,23524.91,34106.30,34106.30
1,100,10,64,20,2154.06,2000,1280000,928,0.567,106520.72,131965.07,131965.07
2,1000,10,64,20,18615.74,20000,12800000,1074,0.656,932106.07,1094010.36,1094010.36
|
/request-review @aicam |
aicam
left a comment
aicam
left a comment
There was a problem hiding this comment.
Please add a new column to workflow_computing_unit which is "termination_reason" and we should have two types of termination (as of this PR):
- USER_REQUESTED
- GARBAGE_COLLECTED
aicam
left a comment
aicam
left a comment
There was a problem hiding this comment.
Please add a new column to workflow_computing_unit which is "termination_reason" and we should have two types of termination (as of this PR):
- USER_REQUESTED
- GARBAGE_COLLECTED
…ave been closed. 3). use DSL.max instead
136f725 to
a11601f
Compare
|
this video shows the new effect: texera-PR6046.mp4 |
|
@yrenat please also have more tests. the coverage is too low. |
github-actions
Bot
added
the
dependencies
done. The coverage is now 100% |
| @Produces(Array(MediaType.APPLICATION_JSON)) | ||
| @Path("/computing-unit") | ||
| class ComputingUnitManagingResource { | ||
| private val logger = LoggerFactory.getLogger(classOf[ComputingUnitManagingService]) |
There was a problem hiding this comment.
With this setup, the log line in terminateComputingUnit will emit under the service's logger name instead of the resource's (which is the class it is created under). I recommend using the resource class instead here:
| private val logger = LoggerFactory.getLogger(classOf[ComputingUnitManagingService]) | |
| private val logger = LoggerFactory.getLogger(classOf[ComputingUnitManagingResource]) |
| val cutoff = new Timestamp(now.getTime - cleanupConfig.idleTimeoutMinutes * 60 * 1000) | ||
| val activeStatuses = Seq(Short.box(0), Short.box(1), Short.box(2)) | ||
|
|
||
| withTransaction(context) { ctx => |
There was a problem hiding this comment.
A single DB transaction appears to handle the full scan and deletion of pods. I recommend deleting pods outside the transaction or committing per unit, so one failure does not undo the whole batch.
github-actions
Bot
added
frontend
65c2db2 to
e551284
Compare
github-actions
Bot
added
dependencies
…-cus # Conflicts: # build.sbt # computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala # computing-unit-managing-service/src/test/scala/org/apache/texera/service/ComputingUnitManagingServiceRunSpec.scala # computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala # sql/texera_ddl.sql # sql/updates/28.sql
534bdad to
522a05d
Compare
github-actions
Bot
removed
the
dependencies
6 participants
What changes were proposed in this PR?
Following discussion #6264, this PR adds backend-side cleanup for idle Kubernetes computing units.
The main change is a scheduled cleanup task in the computing unit managing service that periodically scans active Kubernetes computing units and terminates units that have been inactive longer than a configurable timeout.
The implementation includes the following changes:
ComputingUnitManagingServicethat runs the idle cleanup logic at a fixed interval.ComputingUnitManagingResource:The timeout and check interval are configurable through environment variables, so the behavior can be tuned for different deployment or testing needs without modifying the code.
Any related issues, documentation, discussions?
Fixes #5362
How was this PR tested?
Tested locally on the Kubernetes deployment flow.
fix-idle-CU-demo.mp4
Was this PR authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex GPT-5