Skip to content

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

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

Topic multi-partition writer (write-by-key)#864
vgvoleg wants to merge 7 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, reusing a per-partition writer under the hood.

  • Choosers: TopicWriterPartitionByKeyKafka (murmur2, Kafka-compatible) and TopicWriterPartitionByKeyBound (server key-range); adaptive default per topic.
  • Transparent auto-partition split handling: detects the split via the OVERLOADED write-stream error, re-describes, and resends un-acked messages to the child partitions — no loss, no duplicates. Transient overloads recover in place.
  • Idle per-partition sub-writers are evicted after a timeout and recreated on demand.
  • Exposes partition key_range on describe_topic.
  • Sync + async facades.

Split path validated live against a cloud cluster (exactly-once across real splits); merge is not yet server-supported.

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
Comment thread ydb/_topic_writer/topic_writer_multi_asyncio.py
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
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.53%. Comparing base (c8db8d0) to head (f838f42).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #864      +/-   ##
==========================================
+ Coverage   82.36%   83.53%   +1.16%     
==========================================
  Files          99      102       +3     
  Lines       12753    13475     +722     
  Branches     1242     1349     +107     
==========================================
+ Hits        10504    11256     +752     
+ Misses       1794     1764      -30     
  Partials      455      455              
Flag Coverage Δ
integration 79.11% <55.46%> (-1.11%) ⬇️
unit 51.59% <100.00%> (+3.02%) ⬆️

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.64% <100.00%> (+1.31%) ⬆️
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_asyncio.py 100.00% <100.00%> (ø)
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%) ⬆️

... and 2 files with indirect coverage changes

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

- 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
@vgvoleg vgvoleg added the SLO label Sep 4, 2026
@github-actions github-actions Bot removed the SLO label Sep 4, 2026
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.
@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
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.
@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
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 7, 2026
@github-actions github-actions Bot removed the SLO label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

🌋 SLO Test Results

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

Commit: 27d25e0 · View run

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

Threshold violations:

async-topic:

  • write_retry_attempts: ▲ 25.0% (≥ 20% warn)

sync-table:

  • read_retry_attempts: ▲ 66.7% (≥ 50% fail)
  • write_retry_attempts: ▲ 60.0% (≥ 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