Skip to content

chore: remove dead table-service transaction machinery - #595

Open
sshaplygin wants to merge 1 commit into
ydb-platform:masterfrom
sshaplygin:chore/remove-dead-table-tx
Open

chore: remove dead table-service transaction machinery#595
sshaplygin wants to merge 1 commit into
ydb-platform:masterfrom
sshaplygin:chore/remove-dead-table-tx

Conversation

@sshaplygin

Copy link
Copy Markdown
Contributor

Part of #518.

Context

#518 asks to remove the old transaction API once the query service can span tables and topics (#340). Checking the current state before deleting anything:

  • feat: query service: transactions with tables and topics #340 is closed (2026-07-07), so the stated blocker is gone.
  • The public side of the removal already happened in the 0.16.0 table/query split. TableClient no longer runs YQL or transactions, there are no #[deprecated] transaction items anywhere, and retry_transaction is already retry_tx.
  • Cross-service transactions work today. Topic transactions are built on the query-service Transaction via QueryTxHookTopicWriterTx::new(.., tx: &mut Transaction), reader.tx_reader(&mut tx), reader.pop_batch_in_tx(&mut tx). All seven integration tests pass against a live server: writer commit/rollback, reader commit/rollback/redelivery/offset-advance, and read_topic_message_in_transaction.

So the API #518 targeted is gone. What remained is the raw plumbing underneath it, which no longer has a caller anywhere in the repository.

What this removes

Removed Why
execute_data_query.rs, transaction_control.rs, commit_transaction.rs, rollback_transaction.rs No callers outside their own module
RawTableClient::{execute_data_query, commit_transaction, rollback_transaction} The only referents of the above
query_stats.rs RawQueryStatMode existed solely to configure ExecuteDataQuery
Operation impls for ExecuteDataQueryResponse, CommitTransactionResponse, RollbackTransactionResponse Orphaned with the methods

323 lines deleted, 4 added.

Two things that look similar but are not touched: the commit_transaction / rollback_transaction calls in client_query/exec.rs are the query service's own in raw_query_service, and the surviving RawTxMode is the query-service type, distinct from the table one removed here.

No public API changes. Every removed item was pub(crate) inside a pub(crate) module and never re-exported.

Verification

Against ydbplatform/local-ydb:nightly, the image CI uses:

cargo test --workspace -- --include-ignored   # 302 passed, 0 failed — same count as master
cargo fmt --check
cargo clippy --workspace --all-targets --no-deps --exclude=ydb-grpc -- -D warnings

Note for reviewers

Two follow-ups I deliberately left alone, since neither is transaction-related and both belong in their own change:

  • RawTableClient::explain_data_query is also uncalled, as is explain_data_query.rs.
  • docker-compose.yaml pins local-ydb:latest, while CI uses :nightly. On :latest, 11 pre-existing integration tests fail with Unknown name: $val.

Whether #518 can now be closed is a maintainer call — this finishes the mechanical part, but the issue was written against a broader API that the 0.16.0 refactor already removed.

🤖 Generated with Claude Code

Part of ydb-platform#518.

ydb-platform#518 asks to remove the old transaction API once the query service can
span tables and topics (ydb-platform#340). ydb-platform#340 landed and closed on 2026-07-07, and
the public side of that removal already happened in the 0.16.0
table/query split: `TableClient` no longer runs YQL or transactions, and
topic transactions are built on the query-service `Transaction` via
`QueryTxHook`.

What remained was the raw plumbing underneath, which no longer has a
caller anywhere in the repository:

- `execute_data_query.rs`, `transaction_control.rs`,
  `commit_transaction.rs`, `rollback_transaction.rs`
- the matching `RawTableClient::{execute_data_query, commit_transaction,
  rollback_transaction}` methods
- `query_stats.rs`, whose `RawQueryStatMode` existed only to configure
  `ExecuteDataQuery`
- the `Operation` impls for `ExecuteDataQueryResponse`,
  `CommitTransactionResponse` and `RollbackTransactionResponse`

The `commit_transaction` / `rollback_transaction` calls that remain in
`client_query/exec.rs` are the query service's own, in
`raw_query_service`; likewise the surviving `RawTxMode` is the query
service type, unrelated to the table one removed here.

No public API changes: every removed item was `pub(crate)` inside a
`pub(crate)` module and never re-exported.

Verified against ydbplatform/local-ydb:nightly: 302 tests pass with
--include-ignored, matching the count on master. The seven cross-service
transaction tests (topic writer/reader commit, rollback, redelivery,
offset advance) pass unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.39%. Comparing base (a6d7911) to head (375ff38).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #595      +/-   ##
==========================================
+ Coverage   86.91%   87.39%   +0.48%     
==========================================
  Files         198      193       -5     
  Lines       19492    19373     -119     
==========================================
- Hits        16941    16931      -10     
+ Misses       2551     2442     -109     
Flag Coverage Δ
rust-1.88.0 87.39% <ø> (+0.48%) ⬆️
rust-1.96.1 87.68% <ø> (+0.53%) ⬆️
tests 87.39% <ø> (+0.48%) ⬆️
ubuntu 87.39% <ø> (+0.48%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

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.

1 participant