Skip to content

Make write threads for applier configurable#549

Merged
morgo merged 6 commits intoblock:mainfrom
morgo:mtocker-write-threads-configuration
Jan 5, 2026
Merged

Make write threads for applier configurable#549
morgo merged 6 commits intoblock:mainfrom
morgo:mtocker-write-threads-configuration

Conversation

@morgo
Copy link
Collaborator

@morgo morgo commented Jan 5, 2026

A Pull Request should be associated with an Issue.

We wish to have discussions in Issues. A single issue may be targeted by multiple PRs.
If you're offering a new feature or fixing anything, we'd like to know beforehand in Issues,
and potentially we'll be able to point development in a particular direction.
Further notes in https://github.com/block/spirit/blob/main/.github/CONTRIBUTING.md

Previously we used 40 write threads regardless. This is going to be a problem when running concurrent tests in CI (the appliers are still experimental, so there aren't that many tests for them yet).

I've changed it to be configurable. This changes the signature of NewSingleTargetApplier such that it can return errors. This is the same as NewShardedApplier.

@morgo morgo requested a review from nonbb January 5, 2026 18:41
TargetDSN string `name:"target-dsn" help:"Where to copy the tables to." default:"spirit:spirit@tcp(127.0.0.1:3306)/dest"`
TargetChunkTime time.Duration `name:"target-chunk-time" help:"How long each chunk should take to copy" default:"5s"`
Threads int `name:"threads" help:"How many chunks to copy in parallel" default:"2"`
WriteThreads int `name:"write-threads" help:"How many concurrent write threads to use per target" default:"2"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

would it be apparent when errors pop up that we'd need to tweak the default threads up here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i.e. tests timing out/slow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's was not obvious why the tests were failing, but I reduced the concurrency and it worked.

This is similar to scenarios like OOM and CPU load. Programs start performing unreliably.

I tried to consider that it might be something like a race, and thus there's a bug that is fixable. But I don't think that's the case.

@morgo morgo merged commit bdc7f96 into block:main Jan 5, 2026
7 of 8 checks passed
@morgo morgo deleted the mtocker-write-threads-configuration branch January 5, 2026 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants