Skip to content

Topic multi-partition writer (write-by-key) - #864

Open
vgvoleg wants to merge 15 commits into
mainfrom
topic-multiwriter
Open

Topic multi-partition writer (write-by-key)#864
vgvoleg wants to merge 15 commits into
mainfrom
topic-multiwriter

Conversation

@vgvoleg

@vgvoleg vgvoleg commented Jul 21, 2026

Copy link
Copy Markdown
Member

Adds topic_client.multiwriter(...): one logical writer that routes messages across partitions by their key, with synchronous and asynchronous APIs.

  • Choosers: TopicWriterPartitionByKeyKafka (Kafka-compatible murmur2) and TopicWriterPartitionByKeyBound (server key ranges), with an adaptive default per topic.
  • On partition changes, reconciles unfinished writes against each retired producer's persisted sequence number and resends the remaining messages to active descendants. Handles cascaded splits and merge topologies; merged tails are sent in sequence-number order before new writes.
  • Supports automatic and manual sequence numbers in one increasing sequence across the multiwriter. Accepted numbers are preserved during retries and partition changes.
  • Recovers transient overloads in place and reports unrecoverable delivery errors through write results, flush, and close.
  • Evicts idle partition writers and recreates them on demand. Initialization supports both driver types and survives cancellation of an individual waiter.
  • Exposes partition key_range on describe_topic.

Client-side merge handling does not enable server-side automatic merge; its availability depends on the target server.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a new “multi-partition” topic writer API that routes messages across partitions by message key, while reusing the existing per-partition writer implementation (buffering/encoding/reconnect) and adding split/merge-aware resend logic on top. It also surfaces partition key ranges in topic descriptions to support server-accurate routing on auto-partitioned topics.

Changes:

  • Add topic_client.multiwriter(...) (sync + async), plus partition-chooser implementations (Kafka-hash and server key-range routing) and multi-writer orchestration (split/merge discovery, resend, idle sub-writer eviction).
  • Extend topic DescribeTopic public types to include per-partition key_range.
  • Add unit + integration tests, docs, an example, and a changelog entry for the new feature.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
ydb/topic.py Exposes multiwriter(...) API and re-exports new public types/chooser classes.
ydb/_topic_writer/topic_writer.py Adds message key, split-stopping hook in settings, and internal split error type.
ydb/_topic_writer/topic_writer_test.py Adds golden-vector and chooser behavior tests (murmur, Kafka/bound routing).
ydb/_topic_writer/topic_writer_partition_chooser.py Implements key-based partition choosers and hashing utilities.
ydb/_topic_writer/topic_writer_multi_sync.py Adds sync facade over the async multi-writer.
ydb/_topic_writer/topic_writer_multi_asyncio.py Implements multi-writer routing, per-partition sub-writers, and split/merge resend logic.
ydb/_topic_writer/topic_writer_asyncio.py Adds an internal hook to stop a sub-writer early on split-signaling errors.
ydb/_topic_writer/topic_writer_asyncio_test.py Adds deterministic unit tests for routing, split/merge migration, eviction, and edge cases.
ydb/_grpc/grpcwrapper/ydb_topic.py Maps proto PartitionKeyRange into wrapper/public types for describe results.
ydb/_grpc/grpcwrapper/ydb_topic_public_types.py Adds PartitionKeyRange + PartitionInfo.key_range to the public describe model.
tests/topics/test_topic_writer.py Adds integration tests for key-range exposure and write-by-key behavior (incl. split when it occurs).
examples/topic/multiwriter_example.py Adds a runnable example demonstrating sync/async write-by-key usage.
docs/topic.rst Documents the new write-by-key multiwriter API and chooser semantics.
CHANGELOG.md Adds a user-facing entry for the new multiwriter + key_range exposure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ydb/_topic_writer/topic_writer_partition_chooser.py
Comment thread ydb/_topic_writer/topic_writer_partition_chooser.py Outdated
Comment thread ydb/_topic_writer/topic_writer_partition_chooser.py
Comment thread docs/topic.rst Outdated
Comment thread CHANGELOG.md
Comment thread ydb/topic.py Outdated
Comment thread ydb/topic.py Outdated
Comment thread ydb/_topic_writer/topic_writer_multi_asyncio.py Outdated
Comment thread ydb/_topic_writer/topic_writer_multi_asyncio.py
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.55530% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.66%. Comparing base (5425815) to head (c3fa87c).

Files with missing lines Patch % Lines
ydb/_topic_writer/topic_writer_multi_asyncio.py 96.59% 11 Missing and 10 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #864      +/-   ##
==========================================
+ Coverage   82.59%   83.66%   +1.07%     
==========================================
  Files          99      102       +3     
  Lines       12921    13778     +857     
  Branches     1267     1413     +146     
==========================================
+ Hits        10672    11528     +856     
+ Misses       1798     1789       -9     
- Partials      451      461      +10     
Flag Coverage Δ
integration 79.99% <69.96%> (-0.44%) ⬇️
unit 51.80% <97.32%> (+3.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
ydb/_grpc/grpcwrapper/ydb_topic.py 87.96% <100.00%> (+1.63%) ⬆️
ydb/_grpc/grpcwrapper/ydb_topic_public_types.py 100.00% <100.00%> (ø)
ydb/_topic_writer/topic_writer.py 87.62% <100.00%> (+0.88%) ⬆️
ydb/_topic_writer/topic_writer_asyncio.py 88.98% <100.00%> (+0.07%) ⬆️
ydb/_topic_writer/topic_writer_multi_sync.py 100.00% <100.00%> (ø)
...db/_topic_writer/topic_writer_partition_chooser.py 100.00% <100.00%> (ø)
ydb/topic.py 89.88% <100.00%> (+3.86%) ⬆️
ydb/_topic_writer/topic_writer_multi_asyncio.py 96.59% <96.59%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vgvoleg vgvoleg added the SLO label Sep 4, 2026
@github-actions github-actions Bot removed the SLO label Sep 4, 2026
@vgvoleg vgvoleg added the SLO label Sep 4, 2026
@github-actions github-actions Bot removed the SLO label Sep 4, 2026
@vgvoleg
vgvoleg marked this pull request as ready for review September 4, 2026 12:56
@vgvoleg
vgvoleg requested a review from rekby September 4, 2026 12:59
@vgvoleg vgvoleg added the SLO label Sep 4, 2026
@github-actions github-actions Bot removed the SLO label Sep 4, 2026
@vgvoleg vgvoleg added the SLO label Sep 7, 2026
@github-actions github-actions Bot removed the SLO label Sep 7, 2026
@vgvoleg vgvoleg added the SLO label Sep 7, 2026
@github-actions github-actions Bot removed the SLO label Sep 7, 2026
@vgvoleg
vgvoleg force-pushed the topic-multiwriter branch 2 times, most recently from b118cda to 330a6d7 Compare September 9, 2026 11:32
@vgvoleg vgvoleg added the SLO label Sep 9, 2026
@github-actions github-actions Bot removed the SLO label Sep 9, 2026
@vgvoleg vgvoleg added the SLO label Sep 11, 2026
@github-actions github-actions Bot removed the SLO label Sep 11, 2026
@vgvoleg vgvoleg added the SLO label Sep 11, 2026
@github-actions github-actions Bot removed the SLO label Sep 11, 2026
@vgvoleg vgvoleg added the SLO label Sep 11, 2026
@github-actions github-actions Bot removed the SLO label Sep 11, 2026
Routes messages across partitions by key (Kafka-hash or server
key-range chooser) and transparently resends in-flight messages to
child partitions on an auto-partition split, without loss or
duplicates. Exposes partition key_range on describe_topic.
- take the dedup cut from the server rather than the highest ack we saw:
  an unpinned probe session reads last_seqno across the partition lineage,
  which closes the duplicate window when an ack is lost during a split
- use one writer-wide seqno, so a resend keeps the message's number
- give in-flight messages a terminal outcome when repartition and recovery
  both fail, instead of leaving their futures unresolved
- own repartition tasks: coalesce per partition, cancel and await on close
- fail a migration that would overwrite a colliding seqno
- validate bound chooser ranges after sorting, not in argument order
- raise a validation error, not TopicWriterStopped, for a missing manual seqno
- lower the unclosed TopicClient message to debug
- new `sync-topic-multiwriter` workload: same reader and delivery accounting,
  but writes by key through the multi-partition writer, so chaos runs cover key
  routing and the per-partition sub-writer pool
- one payload stream per key rather than per partition, since a key is what the
  writer keeps ordered
- scope topic paths by workload as well as ref: two topic workloads sharing a
  topic would mix into each other's ordering accounting
- compute the SLO matrix from the changed files so the label starts only the
  affected scenarios; an unrelated diff still runs everything
- tests for the sync facade, the client factories, the key_range wrapper and the
  reconnector error hook, plus the orchestrator's lifecycle and error paths:
  100% of the changed lines are now exercised
- bound chooser: search in place instead of rebuilding a bounds list per message
- Kafka chooser: reject any key range, including the fully open one a single
  auto-partitioned partition reports before its first split
- drop an unreachable guard in the migration fail-tail loop
- document that the dedup cut comes from the server, and fix a comment typo
Codecov counts partially taken branches, not just unexecuted lines, so the
patch was short of 100% even with every line covered. Adds the missing sides:
the guards against completing a caller's future twice, the synchronous-driver
describe path, idle eviction switched off, and the coverage check rejecting
children that overlap instead of extending the parent's range.

Also maps SLO harness paths to the workloads they actually serve, so touching
the topic jobs no longer starts the table and query scenarios.
The metric is zero almost everywhere and spikes only around a chaos fault, so
comparing two such runs divides one accidental spike by another: across two runs
it flagged sync-table (50%, sums 12 vs 14) and then async-topic (450%, sums 75 vs
12), each from 2 non-zero samples out of 120 with a concordance of 0.008.

`direction: neutral` disables exactly that comparison while the absolute check
still runs, so "retries must stay at zero" is kept. The absolute bound is
restated because an exact-name entry replaces the action's `*_attempts` default
outright rather than merging with it.
Walking up to the ancestors was copied from the reference implementations, but
it is unnecessary here and unsafe once merges exist.

Unnecessary because every move is already gated by a cut of at least that
producer's server seqno, so a message that travelled has a number above it and a
retired producer never grows. Unsafe because a merge child has two parents whose
branches numbered independently: the maximum over them pulls in the sibling's
history, which says nothing about messages that came down this branch and can be
arbitrarily higher, marking unsent messages as written. Neither reference hits
this — both read only the first parent and assume a single one.

Removing the walk also removes the parent map it needed, and with it the case
where a stable producer id prefix let a previous run's ancestor inflate the cut.

Adds the architecture document the change is described in, in English and
Russian.
@vgvoleg vgvoleg added the SLO label Sep 11, 2026
@github-actions github-actions Bot removed the SLO label Sep 11, 2026
@github-actions

Copy link
Copy Markdown

🌋 SLO Test Results

🔴 6 workload(s) tested — 3 workload(s) exceeded failure thresholds

Commit: 0d81151 · View run

Workload Thresholds Duration Report
sync-topic 🔴 Failure 10m 7s 📄 Report
sync-topic-multiwriter 🔴 Failure 10m 7s 📄 Report
sync-table 🔴 Failure 10m 3s 📄 Report
async-topic 🟢 OK 10m 2s 📄 Report
async-query 🟢 OK 10m 3s 📄 Report
sync-query 🟢 OK 10m 3s 📄 Report

Threshold violations:

sync-topic:

  • read_retry_attempts: ▲ 400.0% (≥ 50% fail)

sync-topic-multiwriter:

  • read_retry_attempts: ▲ 100.0% (≥ 50% fail)

sync-table:

  • write_retry_attempts: ▲ 73.3% (≥ 50% fail)

Generated by ydb-slo-action

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