Skip to content

Commit 48a9b93

Browse files
authored
[query|batch] collect partition results as they complete (#15316)
## Change Description Modifies the query `ServiceBackend` to collect partititon results incrementally, instead of waiting for the job group to complete. It does this by fetching succeeded jobs whose `end_time` is greater than the maximum from the previous batch of jobs. The batch service front end needed the following modifications: - Include the job's `end_time` in http responses via `JobListEntryV1Alpha` (and consequently `GetJobResponseV1Alpha`) - Include milliseconds in date time string format (previously truncated to seconds). The first has the performance implication of always joining with the attempts table. I see no way around this without a more significant change to enable this new query-on-batch capability. The `JobEndTimeQuery` has been simplified as a consequence. Driver memory usage has significantly reduced as a consequence of this change. I've included before and after screenshots of the driver's cpu and memory usage in a stress test of force-counting a range table with 10,000 rows and partitions. Before this change, the driver needed 21GB to collect these results. This change reduces that to <2GB meaning it can now run on a standard worker. ![image.png](https://app.graphite.com/user-attachments/assets/fdf21f80-cb69-40f4-b60d-a9b32eb1a8bb.png) ![image.png](https://app.graphite.com/user-attachments/assets/89dfb8ad-23cc-4fad-85be-034cb5f95d18.png) Through this testing, I discovered that `end_time` isn't a very good index for incrementally collecting jobs. Due to the lack of ordering guarantees on attempt `end_time`, I noticed a significant number of jobs were never returned by the batch service while using a polling interval of `[500, 5000]` ms. Doubling this interval reduced the number of stragglers by half which is somewhat intuitive. I don't think increasing futher is a good solution; instead we should consider other ways to get jobs we haven't processed. This change reduces the maximum number of parallel reads from 1000 to 50. 50 is the page size batch uses in job responses. After experimenting a bit I found out that, for this test at least, increasing the parallelism didn't decrease wall clock time, only memory consumption. Fixes #15288, #14607 ## Security Assessment This change potentially impacts the Hail Batch instance as deployed by Broad Institute in GCP ### Impact Rating This change has a low security impact ### Impact Description The changes primarily involve database query optimizations and internal API enhancements for job result collection. The modifications add time-based filtering capabilities and improve query efficiency without introducing new external interfaces or changing authentication/authorization mechanisms. All changes maintain existing data validation and access control patterns. ### Appsec Review - [ ] Required: The impact has been assessed and approved by appsec
1 parent 566f39f commit 48a9b93

13 files changed

Lines changed: 501 additions & 297 deletions

File tree

batch/batch/batch.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,11 @@ def job_record_to_dict(record: Dict[str, Any], name: Optional[str]) -> JobListEn
138138
exit_code = None
139139
duration = None
140140

141-
if record['cost_breakdown'] is not None:
142-
record['cost_breakdown'] = cost_breakdown_to_dict(json.loads(record['cost_breakdown']))
141+
cost_breakdown = record['cost_breakdown']
142+
cost_breakdown = cost_breakdown_to_dict(json.loads(cost_breakdown)) if cost_breakdown is not None else None
143+
144+
end_time = record['end_time']
145+
end_time = time_msecs_str(end_time) if end_time is not None else None
143146

144147
return cast(
145148
JobListEntryV1Alpha,
@@ -151,10 +154,11 @@ def job_record_to_dict(record: Dict[str, Any], name: Optional[str]) -> JobListEn
151154
'billing_project': record['billing_project'],
152155
'state': record['state'],
153156
'exit_code': exit_code,
157+
'end_time': end_time,
154158
'duration': duration,
155159
'cost': coalesce(record['cost'], 0),
156160
'msec_mcpu': record['msec_mcpu'],
157-
'cost_breakdown': record['cost_breakdown'],
161+
'cost_breakdown': cost_breakdown,
158162
'always_run': bool(record['always_run']),
159163
'display_state': None,
160164
},

batch/batch/front_end/front_end.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2327,7 +2327,13 @@ async def _get_job(app, batch_id, job_id) -> GetJobResponseV1Alpha:
23272327
record = await db.select_and_fetchone(
23282328
"""
23292329
WITH base_t AS (
2330-
SELECT jobs.*, user, billing_project, ip_address, format_version, t.attempt_id AS last_cancelled_attempt_id
2330+
SELECT jobs.*
2331+
, user
2332+
, billing_project
2333+
, ip_address
2334+
, format_version
2335+
, t.attempt_id AS last_cancelled_attempt_id
2336+
, attempts.end_time
23312337
FROM jobs
23322338
INNER JOIN batches
23332339
ON jobs.batch_id = batches.id

batch/batch/front_end/query/query.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -273,11 +273,7 @@ def __init__(self, operator: ComparisonOperator, time_msecs: int):
273273

274274
def query(self) -> Tuple[str, List[int]]:
275275
op = self.operator.to_sql()
276-
sql = f"""
277-
((jobs.batch_id, jobs.job_id) IN
278-
(SELECT batch_id, job_id FROM attempts
279-
WHERE end_time {op} %s))
280-
"""
276+
sql = f'(latest_attempt.end_time {op} %s)'
281277
return (sql, [self.time_msecs])
282278

283279

batch/batch/front_end/query/query_v1.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,11 +267,19 @@ def parse_job_group_jobs_query_v1(
267267
sql = f"""
268268
WITH base_t AS
269269
(
270-
SELECT jobs.*, batches.user, batches.billing_project, batches.format_version,
271-
job_attributes.value AS name
270+
SELECT jobs.*
271+
, batches.user
272+
, batches.billing_project
273+
, batches.format_version
274+
, job_attributes.value AS name
275+
, latest_attempt.end_time
272276
FROM jobs
273277
INNER JOIN batches ON jobs.batch_id = batches.id
274278
INNER JOIN batch_updates ON jobs.batch_id = batch_updates.batch_id AND jobs.update_id = batch_updates.update_id
279+
LEFT JOIN attempts AS latest_attempt
280+
ON jobs.batch_id = latest_attempt.batch_id
281+
AND jobs.job_id = latest_attempt.job_id
282+
AND jobs.attempt_id = latest_attempt.attempt_id
275283
LEFT JOIN job_attributes
276284
ON jobs.batch_id = job_attributes.batch_id AND
277285
jobs.job_id = job_attributes.job_id AND

batch/batch/front_end/query/query_v2.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,15 +277,25 @@ def parse_job_group_jobs_query_v2(
277277
where_args += args
278278

279279
sql = f"""
280-
SELECT STRAIGHT_JOIN jobs.*, batches.user, batches.billing_project, batches.format_version, job_attributes.value AS name, cost_t.cost,
281-
cost_t.cost_breakdown
280+
SELECT STRAIGHT_JOIN jobs.*
281+
, batches.user
282+
, batches.billing_project
283+
, batches.format_version
284+
, job_attributes.value AS name
285+
, cost_t.cost
286+
, cost_t.cost_breakdown
287+
, latest_attempt.end_time
282288
FROM jobs
283289
INNER JOIN batches ON jobs.batch_id = batches.id
284290
INNER JOIN batch_updates ON jobs.batch_id = batch_updates.batch_id AND jobs.update_id = batch_updates.update_id
285291
LEFT JOIN job_attributes
286292
ON jobs.batch_id = job_attributes.batch_id AND
287293
jobs.job_id = job_attributes.job_id AND
288294
job_attributes.`key` = 'name'
295+
LEFT JOIN attempts AS latest_attempt
296+
ON jobs.batch_id = latest_attempt.batch_id
297+
AND jobs.job_id = latest_attempt.job_id
298+
AND jobs.attempt_id = latest_attempt.attempt_id
289299
LEFT JOIN LATERAL (
290300
SELECT COALESCE(SUM(`usage` * rate), 0) AS cost, JSON_OBJECTAGG(resources.resource, COALESCE(`usage` * rate, 0)) AS cost_breakdown
291301
FROM (SELECT resource_id, CAST(COALESCE(SUM(`usage`), 0) AS SIGNED) AS `usage`

0 commit comments

Comments
 (0)