Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: enable wal reader fanout in tests #10301

Merged
merged 1 commit into from
Jan 24, 2025
Merged

Conversation

VladLazar
Copy link
Contributor

@VladLazar VladLazar commented Jan 7, 2025

Note: this has to merge after the release is cut on 2025-01-17 for compat tests to start passing.

Problem

SK wal reader fan-out is not enabled in tests by default.

Summary of changes

Enable it.

@VladLazar VladLazar added the run-benchmarks Indicates to the CI that benchmarks should be run for PR marked with this label label Jan 7, 2025
Copy link

github-actions bot commented Jan 7, 2025

7531 tests run: 7136 passed, 0 failed, 395 skipped (full report)


Flaky tests (5)

Postgres 17

Postgres 16

Postgres 14

Code coverage* (full report)

  • functions: 33.5% (8465 of 25279 functions)
  • lines: 49.2% (70962 of 144156 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
bacb773 at 2025-01-24T11:04:02.837Z :recycle:

@VladLazar
Copy link
Contributor Author

Note to self: the only "real" failure here is test_sharding_split_failures[failure2]. The compat failures are due to the currently released SK version not having the --wal-reader-fanout flag.

@VladLazar VladLazar force-pushed the vlad/sk-fanout-test-enable branch from 3fdf4f6 to 0b1eb70 Compare January 8, 2025 18:34
@VladLazar VladLazar force-pushed the vlad/sk-fanout-test-enable branch from 0b1eb70 to abc511d Compare January 13, 2025 18:48
@VladLazar VladLazar force-pushed the vlad/sk-fanout-test-enable branch from abc511d to aa5e83e Compare January 14, 2025 17:00
@VladLazar VladLazar force-pushed the vlad/sk-fanout-test-enable branch from aa5e83e to 9dc10b8 Compare January 15, 2025 11:26
github-merge-queue bot pushed a commit that referenced this pull request Jan 15, 2025
## Problem

Safekeepers currently decode and interpret WAL for each shard
separately.
This is wasteful in terms of CPU memory usage - we've seen this in
profiles.

## Summary of changes

Fan-out interpreted WAL to multiple shards. 
The basic is that wal decoding and interpretation happens in a separate
tokio task and senders
attach to it. Senders only receive batches concerning their shard and
only past the Lsn they've last seen.

Fan-out is gated behind the `wal_reader_fanout` safekeeper flag
(disabled by default for now).

When fan-out is enabled, it might be desirable to control the absolute
delta between the
current position and a new shard's desired position (i.e. how far behind
or ahead a shard may be).
`max_delta_for_fanout` is a new optional safekeeper flag which dictates
whether to create a new
WAL reader or attach to the existing one. By default, this behaviour is
disabled. Let's consider enabling
it if we spot the need for it in the field.

## Testing

Tests passed [here](#10301)
with wal reader fanout enabled
as of
34f6a71.

Related: #9337
Epic: #9329
Base automatically changed from vlad/sk-fanout to main January 15, 2025 15:34
@VladLazar VladLazar force-pushed the vlad/sk-fanout-test-enable branch from 9dc10b8 to 2fd1f51 Compare January 15, 2025 17:53
@VladLazar VladLazar force-pushed the vlad/sk-fanout-test-enable branch from 2fd1f51 to bacb773 Compare January 22, 2025 13:46
@VladLazar VladLazar marked this pull request as ready for review January 23, 2025 15:49
@VladLazar VladLazar enabled auto-merge January 24, 2025 10:34
@VladLazar VladLazar added this pull request to the merge queue Jan 24, 2025
Merged via the queue into main with commit de82764 Jan 24, 2025
89 checks passed
@VladLazar VladLazar deleted the vlad/sk-fanout-test-enable branch January 24, 2025 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-benchmarks Indicates to the CI that benchmarks should be run for PR marked with this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants