Skip to content

Commit

Permalink
fix(pageserver): extend split job key range to the end (#10484)
Browse files Browse the repository at this point in the history
## Problem

Not really a bug fix, but hopefully can reproduce
#10482 more.

If the layer map does not contain layers that end at exactly the end
range of the compaction job, the current split algorithm will produce
the last job that ends at the maximum layer key. This patch extends it
all the way to the compaction job end key.

For example, the user requests a compaction of 0000...FFFF. However, we
only have a layer 0000..3000 in the layer map, and the split job will
have a range of 0000..3000 instead of 0000..FFFF.

This is not a correctness issue but it would be better to fix it so that
we can get consistent job splits.

## Summary of changes

Compaction job split will always cover the full specified key range.

Signed-off-by: Alex Chi Z <chi@neon.tech>
  • Loading branch information
skyzh authored Jan 23, 2025
1 parent 0af40b5 commit 92d95b0
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
6 changes: 6 additions & 0 deletions pageserver/src/tenant/timeline/compaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2212,6 +2212,12 @@ impl Timeline {
} else {
end
};
let end = if ranges_num == idx + 1 {
// extend the compaction range to the end of the key range if it's the last partition
end.max(job.compact_key_range.end)
} else {
end
};
info!(
"splitting compaction job: {}..{}, estimated_size={}",
start, end, total_size
Expand Down
9 changes: 5 additions & 4 deletions test_runner/regress/test_compaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,7 @@ def test_pageserver_gc_compaction_smoke(neon_env_builder: NeonEnvBuilder, with_b
child_workloads: list[Workload] = []

for i in range(1, churn_rounds + 1):
if i % 10 == 0:
log.info(f"Running churn round {i}/{churn_rounds} ...")
log.info(f"Running churn round {i}/{churn_rounds} ...")
if i % 10 == 5 and with_branches == "with_branches":
branch_name = f"child-{i}"
branch_timeline_id = env.create_branch(branch_name)
Expand All @@ -172,8 +171,10 @@ def test_pageserver_gc_compaction_smoke(neon_env_builder: NeonEnvBuilder, with_b
"sub_compaction_max_job_size_mb": 16,
},
)

workload.churn_rows(row_count, env.pageserver.id)
# do not wait for upload so that we can see if gc_compaction works well with data being ingested
workload.churn_rows(row_count, env.pageserver.id, upload=False)
time.sleep(1)
workload.validate(env.pageserver.id)

def compaction_finished():
queue_depth = len(ps_http.timeline_compact_info(tenant_id, timeline_id))
Expand Down

1 comment on commit 92d95b0

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7521 tests run: 7125 passed, 0 failed, 396 skipped (full report)


Flaky tests (7)

Postgres 17

Postgres 16

Postgres 15

Postgres 14

Code coverage* (full report)

  • functions: 33.5% (8495 of 25340 functions)
  • lines: 49.3% (71391 of 144707 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
92d95b0 at 2025-01-23T02:51:34.620Z :recycle:

Please sign in to comment.