This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 6f0d90a5848 [chore](row-binlog) Remove dead code and reserve retired
field ids (#69238)
6f0d90a5848 is described below
commit 6f0d90a5848cae80db81af790f17f54ea3214911
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Mon Oct 19 08:27:02 2026 +0100
[chore](row-binlog) Remove dead code and reserve retired field ids (#69238)
### What problem does this PR solve?
Issue Number: None
Related PR: #62058, #63643, #63850, #64133, #65810
Problem Summary:
**In short.** Row binlog (`binlog.format = 'ROW'`) was reworked several
times on master. The reworks left dead code behind, and they deleted
thrift / protobuf fields without retiring their ids. This PR deletes the
dead code and retires the ids: `reserved` in protobuf, a `// N:
deprecated <name>` comment in thrift. After this, no later change can
reuse an id that an older master build may have persisted or sent.
Nothing that runs today changes. This is also groundwork for renaming
row binlog to changelog: code deleted here does not have to be renamed
there.
**Background.**
- #62058 and #63643 first kept row binlog inside the base tablet. The
tablet meta carried its own binlog schema and rowset metas
(`row_binlog_schema`, `row_binlog_rs_metas`, `row_binlog_schema_hash`).
Delete bitmaps had an `is_binlog_delvec` flag. Clone and snapshot
requests had a `copy_type`. Tablet reports carried `binlog_size` /
`binlog_file_num`, which the FE kept on `Replica`.
- #63850 added `binlog_timestamp_col_idx` / `binlog_lsn_col_idx` to the
tablet schema. #64133 replaced them with `binlog_{tso,lsn,op}_col_idx`
under new ids (41-43, and 42-44 in the cloud message). So the name
`binlog_lsn_col_idx` is still in use, but under a new id.
- #65810 moved row binlog into an independent companion tablet. Most of
the per-tablet fields above were deleted then. #65810 also added a
thrift `TTabletWithPartition.binlog_tablet_id`, which nothing uses.
**The problem.**
- Ten protobuf ids and six thrift ids were deleted without being
retired. `gensrc/AGENTS.md` requires a deleted id to be retired so it is
never reused. Protobuf is the risky case: these messages are persisted
in tablet meta (local RocksDB, and the cloud meta-service). Suppose a
later field took one of these ids with another type or meaning. A tablet
meta written by an older master build could then be decoded into the new
field.
- Some code that nothing reaches is still there. FE `Replica` still
carries `binlogSize` / `binlogFileNum` and writes them into the image,
though nothing has set them since #65810. The thrift
`TTabletWithPartition.binlog_tablet_id` was never used. The BE has
unused constants, a method and two error codes. Without this PR, the
changelog rename would have to rename all of it.
**How this PR fixes it.**
Dead code removed:
| Item | Why it is dead |
|---|---|
| thrift `TTabletWithPartition.binlog_tablet_id` (3) |
`TTabletWithPartition` only lives inside the BE (`VTabletWriter`); field
3 is never set or read. The load path carries the companion tablet id in
the protobuf `PTabletWithPartition.binlog_tablet_id`, which stays. |
| FE `Replica.binlogSize` / `binlogFileNum` (Gson `bs` / `bfn`) and
their getters / setters | Their only writers, the tablet report fields,
were removed in #65810; nothing reads them. Gson skips the two keys when
it loads an image written by an older master. |
| BE `kRowBinlogPrefix`, `FDRowBinlogSuffix`,
`Tablet::enable_row_binlog()` | No references. |
| BE error codes `BINLOG_COMPACTION_INVALID_PARAMETERS` (-2011),
`BINLOG_COMPACTION_NO_SUITABLE_VERSION` (-2012) | No references. Removed
the same way -2007 was, without a placeholder. |
Retired ids:
| File | Message | Ids | Old fields |
|---|---|---|---|
| olap_file.proto | `TabletMetaPB` | 42, 43, 44 | `row_binlog_schema`,
`row_binlog_rs_metas`, `row_binlog_schema_hash` |
| olap_file.proto | `TabletMetaCloudPB` | 43, 44 |
`row_binlog_rs_metas`, `row_binlog_schema_hash` |
| olap_file.proto | `TabletSchemaPB` / `TabletSchemaCloudPB` | 37, 38 /
38, 39 | `binlog_timestamp_col_idx`, `binlog_lsn_col_idx` (the old ids
only; see Background) |
| olap_file.proto | `DeleteBitmapPB` | 5 | `is_binlog_delvec` |
| AgentService.thrift | `TCloneReq` / `TSnapshotRequest` | 14 / 15 |
`copy_type` |
| BackendService.thrift | `TTabletStat` | 12, 13 | `binlog_size`,
`binlog_file_num` |
| MasterService.thrift | `TTabletInfo` | 26, 27 | `binlog_size`,
`binlog_file_num` |
| PaloInternalService.thrift | `TTabletWithPartition` | 3 |
`binlog_tablet_id` |
None of these ids is used on branch-4.2, branch-4.1, branch-4.0 or
branch-3.1 (checked with `git show` on each branch), so retiring them
agrees with every release. `TabletMetaCloudPB` 42 is not in the table:
it already means something else on master (it was `row_binlog_schema`,
it is now `binlog_tablet_id`). This PR cannot undo that.
Not included: `TTabletSchema.binlog_{tso,lsn,op}_idx` (28-30). The FE
stopped sending them in #65810, but BE unit tests still set them when
they create a companion tablet. Whether production needs them is a
separate question, left to a follow-up.
**Results.**
- No behavior change. The one thrift field deleted here was never set,
the other thrift changes are comments, and `reserved` only makes protoc
reject a future reuse of the id.
- A peer built from an older master that still sends one of the deleted
thrift fields has it skipped as an unknown field, as it already is
today. An FE image that still has `bs` / `bfn` loads as before.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Build: `build.sh --fe --be` (BE Release; FE with checkstyle).
- FE unit tests (`run-fe-ut.sh`): `ReplicaTest` (5), `TabletTest` (11),
`OlapTableTest` (18), all pass.
- No BE unit test: nothing under `be/test` references the removed
symbols (`git grep`).
- Cloud was not built locally. It only sees the new `reserved` lines,
which do not change the generated code.
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
---
be/src/common/status.h | 2 --
be/src/storage/binlog.h | 3 ---
be/src/storage/tablet/tablet.h | 4 ----
.../java/org/apache/doris/catalog/Replica.java | 22 ----------------------
gensrc/proto/olap_file.proto | 12 ++++++++++++
gensrc/thrift/AgentService.thrift | 2 ++
gensrc/thrift/BackendService.thrift | 2 ++
gensrc/thrift/MasterService.thrift | 2 ++
gensrc/thrift/PaloInternalService.thrift | 2 +-
9 files changed, 19 insertions(+), 32 deletions(-)
diff --git a/be/src/common/status.h b/be/src/common/status.h
index 11bd06644f0..e3b5c2932f6 100644
--- a/be/src/common/status.h
+++ b/be/src/common/status.h
@@ -250,8 +250,6 @@ namespace ErrorCode {
E(FULL_NO_SUITABLE_VERSION, -2008, false); \
E(FULL_MISS_VERSION, -2009, true); \
E(CUMULATIVE_MEET_DELETE_VERSION, -2010, false); \
- E(BINLOG_COMPACTION_INVALID_PARAMETERS, -2011, true); \
- E(BINLOG_COMPACTION_NO_SUITABLE_VERSION, -2012, false); \
E(META_INVALID_ARGUMENT, -3000, true); \
E(META_OPEN_DB_ERROR, -3001, true); \
E(META_KEY_NOT_FOUND, -3002, false); \
diff --git a/be/src/storage/binlog.h b/be/src/storage/binlog.h
index 83e6ae12007..7c37c4457be 100644
--- a/be/src/storage/binlog.h
+++ b/be/src/storage/binlog.h
@@ -53,7 +53,6 @@ static constexpr int64_t ROW_BINLOG_DELETE = 2;
constexpr std::string_view kBinlogPrefix = "binlog_";
constexpr std::string_view kBinlogMetaPrefix = "binlog_meta_";
constexpr std::string_view kBinlogDataPrefix = "binlog_data_";
-constexpr std::string_view kRowBinlogPrefix = "binlog_row_";
namespace binlog {
@@ -67,8 +66,6 @@ inline std::string build_before_column_name(std::string_view
name) {
} // namespace binlog
constexpr int64_t kBinlogLsnAutoIncId = -1;
-// used in file directory
-constexpr std::string_view FDRowBinlogSuffix = "_row_binlog";
inline auto make_binlog_meta_key(const std::string_view tablet, int64_t
version,
const std::string_view rowset) {
diff --git a/be/src/storage/tablet/tablet.h b/be/src/storage/tablet/tablet.h
index b17a770cef3..012ee6f731c 100644
--- a/be/src/storage/tablet/tablet.h
+++ b/be/src/storage/tablet/tablet.h
@@ -482,10 +482,6 @@ public:
bool enable_ccr_binlog() const {
return enable_binlog() &&
_tablet_meta->binlog_config().is_ccr_binlog_format();
}
- bool enable_row_binlog() const {
- return _tablet_meta->binlog_config().is_enable() &&
- _tablet_meta->binlog_config().is_row_binlog_format();
- }
int64_t binlog_ttl_ms() const { return
_tablet_meta->binlog_config().ttl_seconds(); }
int64_t binlog_max_bytes() const { return
_tablet_meta->binlog_config().max_bytes(); }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java
index 1595cc9f9ec..52db34ceee8 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Replica.java
@@ -94,8 +94,6 @@ public abstract class Replica {
private int schemaHash = -1;
@SerializedName(value = "ds", alternate = {"dataSize"})
private volatile long dataSize = 0;
- @SerializedName(value = "bs", alternate = {"binlogSize"})
- private volatile long binlogSize = 0;
@SerializedName(value = "rc", alternate = {"rowCount"})
private volatile long rowCount = 0;
@SerializedName(value = "st", alternate = {"state"})
@@ -110,10 +108,6 @@ public abstract class Replica {
@SerializedName(value = "lss", alternate = {"localSegmentSize"})
private long localSegmentSize = 0L;
- // Number of binlog files retained on this replica.
- @SerializedName(value = "bfn", alternate = {"binlogFileNum"})
- private volatile long binlogFileNum = -1;
-
public Replica() {
}
@@ -214,22 +208,6 @@ public abstract class Replica {
this.dataSize = dataSize;
}
- public long getBinlogSize() {
- return binlogSize;
- }
-
- public void setBinlogSize(long binlogSize) {
- this.binlogSize = binlogSize;
- }
-
- public long getBinlogFileNum() {
- return binlogFileNum;
- }
-
- public void setBinlogFileNum(long binlogFileNum) {
- this.binlogFileNum = binlogFileNum;
- }
-
public long getRemoteDataSize() {
return 0;
}
diff --git a/gensrc/proto/olap_file.proto b/gensrc/proto/olap_file.proto
index 73403b4b260..4e74d04683e 100644
--- a/gensrc/proto/olap_file.proto
+++ b/gensrc/proto/olap_file.proto
@@ -572,6 +572,9 @@ message TabletSchemaPB {
// storage_format; the fallback path lives in TabletSchema::init_from_pb.
optional TabletStorageFormatPB storage_format = 36;
+ reserved 37; // deprecated binlog_timestamp_col_idx
+ reserved 38; // deprecated binlog_lsn_col_idx
+
// cid of the time-travel commit tso column (__DORIS_COMMIT_TSO_COL__).
optional int32 commit_tso_col_idx = 39 [default = -1];
@@ -634,6 +637,9 @@ message TabletSchemaCloudPB {
// Persisted TStorageFormat (V2=2, V3=3); see
TabletSchemaPB::storage_format.
optional TabletStorageFormatPB storage_format = 37;
+ reserved 38; // deprecated binlog_timestamp_col_idx
+ reserved 39; // deprecated binlog_lsn_col_idx
+
// cid of the time-travel commit tso column (__DORIS_COMMIT_TSO_COL__).
optional int32 commit_tso_col_idx = 40 [default = -1];
@@ -763,6 +769,9 @@ message TabletMetaPB {
optional TabletRolePB tablet_role = 36 [default = TABLET_ROLE_DATA];
// Used by a base tablet to find its paired row-binlog tablet.
optional int64 binlog_tablet_id = 37;
+ reserved 42; // deprecated row_binlog_schema
+ reserved 43; // deprecated row_binlog_rs_metas
+ reserved 44; // deprecated row_binlog_schema_hash
// For cloud
optional int64 index_id = 1000;
@@ -827,6 +836,8 @@ message TabletMetaCloudPB {
optional TabletRolePB tablet_role = 41 [default = TABLET_ROLE_DATA];
// Used by a base tablet to find its paired row-binlog tablet.
optional int64 binlog_tablet_id = 42;
+ reserved 43; // deprecated row_binlog_rs_metas
+ reserved 44; // deprecated row_binlog_schema_hash
// Use for selectdb-cloud
optional string table_name = 101;
@@ -846,6 +857,7 @@ message DeleteBitmapPB {
repeated int64 versions = 3;
// Serialized roaring bitmaps indexed with {rowset_id, segment_id, version}
repeated bytes segment_delete_bitmaps = 4;
+ reserved 5; // deprecated is_binlog_delvec
}
// Local-mode only binlog metadata.
diff --git a/gensrc/thrift/AgentService.thrift
b/gensrc/thrift/AgentService.thrift
index 4ac16bb6c4a..a0cfa3e6eb6 100644
--- a/gensrc/thrift/AgentService.thrift
+++ b/gensrc/thrift/AgentService.thrift
@@ -387,6 +387,7 @@ struct TCloneReq {
11: optional Types.TReplicaId replica_id = 0
12: optional i64 partition_id
13: optional i64 table_id = -1
+ // 14: deprecated copy_type
}
struct TCompactionReq {
@@ -466,6 +467,7 @@ struct TSnapshotRequest {
12: optional Types.TVersion end_version
13: optional bool is_copy_binlog
14: optional Types.TTabletId ref_tablet_id
+ // 15: deprecated copy_type
}
struct TReleaseSnapshotRequest {
diff --git a/gensrc/thrift/BackendService.thrift
b/gensrc/thrift/BackendService.thrift
index 19d80620702..cb7c54291e2 100644
--- a/gensrc/thrift/BackendService.thrift
+++ b/gensrc/thrift/BackendService.thrift
@@ -40,6 +40,8 @@ struct TTabletStat {
9: optional i64 local_segment_size = 0 // .dat
10: optional i64 remote_index_size = 0 // .idx
11: optional i64 remote_segment_size = 0 // .dat
+ // 12: deprecated binlog_size
+ // 13: deprecated binlog_file_num
}
struct TTabletStatResult {
diff --git a/gensrc/thrift/MasterService.thrift
b/gensrc/thrift/MasterService.thrift
index da65873eb6e..43526916b0d 100644
--- a/gensrc/thrift/MasterService.thrift
+++ b/gensrc/thrift/MasterService.thrift
@@ -51,6 +51,8 @@ struct TTabletInfo {
23: optional i64 local_segment_size = 0 // .dat
24: optional i64 remote_index_size = 0 // .idx
25: optional i64 remote_segment_size = 0 // .dat
+ // 26: deprecated binlog_size
+ // 27: deprecated binlog_file_num
// For cloud
1000: optional bool is_persistent
diff --git a/gensrc/thrift/PaloInternalService.thrift
b/gensrc/thrift/PaloInternalService.thrift
index 3df5035abcd..4dc63f53a91 100644
--- a/gensrc/thrift/PaloInternalService.thrift
+++ b/gensrc/thrift/PaloInternalService.thrift
@@ -678,7 +678,7 @@ struct TFoldConstantParams {
struct TTabletWithPartition {
1: required i64 partition_id
2: required i64 tablet_id
- 3: optional i64 binlog_tablet_id
+ // 3: deprecated binlog_tablet_id
}
struct TFetchDataResult {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
