You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "luwei16 (via GitHub)" <gi...@apache.org> on 2026/10/08 15:25:13 UTC
[PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
luwei16 opened a new pull request, #68793:
URL: https://github.com/apache/doris/pull/68793
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: Single-version rowsets store a zero placeholder in their physical commit-TSO column and use rowset metadata to provide the actual timestamp. Local index build/drop and snapshot rowset-ID conversion rebuild metadata without preserving commit_tso. After those operations a fresh reader can return zero instead of the committed timestamp and incorrectly prune rows. Cloud snapshot metadata conversion has the same field omission.
Copy the source commit_tso when present in these three one-to-one rewrite paths, reusing the existing manual-build metadata helper for index changes. Preserve absent and unassigned fields and complete TSO ranges without changing reader compatibility or generic multi-rowset writer behavior. Add tests for build and drop, fresh-reader projection and pruning, persisted local visible/stale rowsets, and Cloud metadata conversion. This prevents future omissions but does not recover already-lost timestamps.
### Release note
Preserve rowset commit timestamps during local index changes and snapshot metadata conversion to avoid incorrect timestamp values and filtering.
### Check List (For Author)
- Test: Unit Test
- ASAN_UT: 8 focused tests reproduced 6 failures before the fix and all passed afterward; all 54 related BE tests passed.
- clang-format v16, check-format, changed-line clang-tidy, build hygiene, and git diff --check.
- No deployment or SQL regression: tests exercise the real rowset conversions, header persistence, and fresh segment readers; live Cloud RESTORE is not validated.
- Behavior changed: Yes (preserve source commit TSO when rebuilding rowset metadata)
- Does this need documentation: No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6064353172
<details>
<summary>ClickBench: <b>Total hot run time: 24.41 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 6fe5c44c417c71c5d82ae4bc22bd6ae90032e785, data reload: false
query1 0.00 0.00 0.01
query2 0.09 0.05 0.05
query3 0.27 0.12 0.12
query4 1.61 0.11 0.11
query5 0.24 0.25 0.24
query6 1.17 0.69 0.69
query7 0.04 0.01 0.01
query8 0.05 0.03 0.04
query9 0.42 0.35 0.35
query10 0.59 0.57 0.57
query11 0.21 0.14 0.14
query12 0.19 0.14 0.14
query13 0.50 0.48 0.49
query14 0.98 0.96 0.96
query15 0.66 0.61 0.61
query16 0.34 0.35 0.35
query17 1.10 1.18 1.13
query18 0.22 0.22 0.23
query19 2.05 1.94 2.00
query20 0.02 0.01 0.02
query21 15.48 0.25 0.14
query22 4.60 0.06 0.05
query23 16.24 0.33 0.13
query24 3.21 0.58 0.38
query25 0.11 0.06 0.04
query26 1.05 0.27 0.16
query27 0.05 0.03 0.03
query28 1.29 0.60 0.41
query29 12.51 4.16 3.24
query30 0.28 0.14 0.14
query31 2.78 0.58 0.34
query32 3.25 0.62 0.51
query33 3.18 3.19 3.20
query34 15.52 4.00 3.38
query35 3.33 3.36 3.37
query36 0.55 0.44 0.42
query37 0.10 0.06 0.07
query38 0.05 0.04 0.04
query39 0.04 0.02 0.02
query40 0.17 0.15 0.14
query41 0.09 0.03 0.02
query42 0.04 0.03 0.03
query43 0.05 0.03 0.03
Total cold run time: 94.72 s
Total hot run time: 24.41 s
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6075213816
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/68793_6fe5c44c417c71c5d82ae4bc22bd6ae90032e785_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/68793_6fe5c44c417c71c5d82ae4bc22bd6ae90032e785_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 76.68% (35190/45892) |
| Line Coverage | 61.86% (397530/642607) |
| Region Coverage | 58.38% (335986/575477) |
| Branch Coverage | 59.23% (154442/260753) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "luwei16 (via GitHub)" <gi...@apache.org>.
luwei16 commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6063253554
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6064096706
<details>
<summary>TPC-H: <b>Total hot run time: 28823 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 6fe5c44c417c71c5d82ae4bc22bd6ae90032e785, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17576 5584 5508 5508
q2 2093 307 251 251
q3 10262 1371 845 845
q4 4675 470 346 346
q5 7434 803 533 533
q6 188 188 156 156
q7 766 819 619 619
q8 9266 1340 1121 1121
q9 5719 4462 4465 4462
q10 6813 1295 1019 1019
q11 430 255 238 238
q12 623 403 285 285
q13 18039 3058 2376 2376
q14 273 272 243 243
q15 q16 736 721 675 675
q17 1161 735 574 574
q18 7169 6234 6153 6153
q19 1108 961 601 601
q20 383 324 231 231
q21 5425 2318 2383 2318
q22 361 315 269 269
Total cold run time: 100500 ms
Total hot run time: 28823 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 6130 6117 6111 6111
q2 674 562 535 535
q3 5212 5386 4953 4953
q4 2024 2142 1574 1574
q5 5293 5172 5966 5172
q6 245 190 164 164
q7 2369 2083 1798 1798
q8 3199 2762 2848 2762
q9 8299 8139 8287 8139
q10 3810 3685 3364 3364
q11 625 420 411 411
q12 710 726 528 528
q13 2918 3296 2536 2536
q14 305 317 283 283
q15 q16 742 726 661 661
q17 8434 7467 7333 7333
q18 13206 12256 13124 12256
q19 912 813 789 789
q20 2194 2178 1950 1950
q21 5893 4803 4808 4803
q22 480 459 390 390
Total cold run time: 73674 ms
Total hot run time: 66512 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6064268451
<details>
<summary>TPC-DS: <b>Total hot run time: 151086 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 6fe5c44c417c71c5d82ae4bc22bd6ae90032e785, data reload: false
query5 4327 564 448 448
query6 429 199 205 199
query7 4805 446 219 219
query8 311 180 168 168
query9 8742 3969 3935 3935
query10 498 322 249 249
query11 5973 3407 3191 3191
query12 150 88 82 82
query13 1239 428 303 303
query14 6495 4767 4483 4483
query14_1 4190 4274 4200 4200
query15 211 194 183 183
query16 933 434 427 427
query17 872 666 550 550
query18 2420 435 305 305
query19 213 162 134 134
query20 82 78 81 78
query21 209 132 125 125
query22 12962 13114 12808 12808
query23 13147 12392 12027 12027
query23_1 12288 12243 12204 12204
query24 7078 824 469 469
query24_1 460 457 451 451
query25 504 381 342 342
query26 1255 252 131 131
query27 2787 434 269 269
query28 4632 1890 1906 1890
query29 1541 567 431 431
query30 288 217 183 183
query31 878 753 643 643
query32 133 83 85 83
query33 492 284 234 234
query34 1003 849 490 490
query35 749 769 655 655
query36 879 784 723 723
query37 124 102 87 87
query38 1792 1746 1680 1680
query39 718 687 693 687
query39_1 643 639 664 639
query40 217 113 105 105
query41 66 64 63 63
query42 84 81 81 81
query43 369 380 317 317
query44 1309 684 688 684
query45 190 182 155 155
query46 856 966 573 573
query47 3010 3038 2929 2929
query48 289 303 213 213
query49 570 393 289 289
query50 677 274 217 217
query51 10524 10381 10150 10150
query52 75 79 69 69
query53 187 208 152 152
query54 224 186 209 186
query55 72 65 67 65
query56 229 212 218 212
query57 1636 1607 1564 1564
query58 287 250 253 250
query59 2351 2380 2094 2094
query60 274 251 235 235
query61 167 170 166 166
query62 406 338 289 289
query63 196 163 162 162
query64 2868 1030 807 807
query65 3417 3372 3364 3364
query66 1786 403 302 302
query67 20200 20087 19869 19869
query68 2915 989 605 605
query69 373 288 245 245
query70 947 843 818 818
query71 290 222 207 207
query72 2757 2559 2300 2300
query73 515 548 292 292
query74 4578 4500 4297 4297
query75 2315 2273 1938 1938
query76 2295 1030 619 619
query77 364 405 299 299
query78 8993 9043 8436 8436
query79 853 884 504 504
query80 558 411 347 347
query81 531 319 270 270
query82 243 133 109 109
query83 304 195 171 171
query84 314 126 94 94
query85 820 413 369 369
query86 313 240 235 235
query87 2010 1973 1820 1820
query88 3624 2687 2667 2667
query89 329 295 256 256
query90 1851 183 175 175
query91 160 152 124 124
query92 96 82 84 82
query93 975 994 553 553
query94 462 299 283 283
query95 585 405 330 330
query96 667 518 230 230
query97 2447 2419 2327 2327
query98 156 148 143 143
query99 787 770 643 643
Total cold run time: 233181 ms
Total hot run time: 151086 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "luwei16 (via GitHub)" <gi...@apache.org>.
luwei16 merged PR #68793:
URL: https://github.com/apache/doris/pull/68793
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "luwei16 (via GitHub)" <gi...@apache.org>.
luwei16 commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6073884799
run external
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "luwei16 (via GitHub)" <gi...@apache.org>.
luwei16 commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6075767735
## Local review of PR #68793
Reviewed the complete six-file three-dot diff from base `038e2274ef678c44f85214e31da6d3e1d48a794f` to head `6fe5c44c417c71c5d82ae4bc22bd6ae90032e785`. Two rounds of main-agent review and three independent full-review subagents per round completed; the final round returned `NO_NEW_VALUABLE_FINDINGS` from all three subagents. Final sweep found no unresolved candidates or Blocker/Major/Minor/Nit findings.
The final round used runtime-recorded `gpt-6.1-sol` at `xhigh`. The first round started at `high` and was interrupted; its recovery used `xhigh`. A complete additional round was performed at `xhigh` rather than relabeling the earlier round. This is a code-review result, not a claim that every CI check has passed.
### Critical checkpoints
| Checkpoint | Conclusion |
|---|---|
| Goal and correctness | The three one-to-one metadata rewrites preserve source commit TSO. Index tests exercise actual placeholder-zero segments, fresh-reader projection and pruning; snapshot tests check converted metadata. |
| Minimality and reuse | Production changes are three guarded copies, nine lines total. Index changes reuse the existing manual-build metadata helper; generic writer/reader behavior is unchanged. |
| Concurrency and locking | Only private output metadata is mutated. Published input rowsets, existing task locks and rowset-update/meta lock order remain unchanged; no new threads, locks or heavyweight operations are introduced. |
| Lifecycle and static initialization | Shared rowset ownership, pending guards and unused-rowset tracking remain intact. The local snapshot's transient rowset is not published; load only changes rowset state. No new static initialization dependency or reference cycle exists. |
| Configuration | No new configuration or dynamic-update behavior. |
| Interface and storage compatibility | No protobuf schema, RPC, symbol or storage-format change. Absent fields remain absent; explicit unassigned values and complete ranges retain their meaning. The reader's -1 compatibility behavior is unchanged. |
| Parallel paths | BUILD/DROP and index formats share the fixed metadata path. Local visible/stale paths share rename; remote rowsets already copy complete PBs. Cloud compaction already sets output TSO ranges, and Cloud PB converters preserve the copied field. |
| Conditions and errors | Presence guards represent valid legacy/prepublish states, not speculative defensive error handling. New test Status/Result accesses are checked before use. |
| Test coverage and negative boundaries | Tests distinguish absent, explicit [-1,-1], assigned single values and snapshot ranges. DROP independently seeds its source TSO; fresh Segment avoids stale-reader masking. Cloud uses nonzero physical segment ID 7. |
| Test results | No handwritten regression outputs. Existing final XML/log evidence was checked: baseline 6 failures of 8, fixed 8/8 passing, related suites 54/54 passing. No tests were rerun during this read-only review. |
| Observability | No new execution stage, error category or distributed operation; existing task/snapshot logs suffice. Additional metrics are not required for a fixed-size metadata copy. |
| Transactions and persistence | Index output enters the existing replacement and production save path. Local TSO is copied before header serialization; Cloud conversion and existing restore PB serialization retain it. No FE EditLog or transaction protocol change. |
| Atomicity, crashes and MoW | Segment content, versions, bitmap remapping, pending guards, replacement, file cleanup and persistence ordering are unchanged; no new partial-commit window is introduced. |
| FE-BE propagation | No new transmitted variable. The existing optional PB field and converters are reused. |
| Performance and memory | Constant-size per-rowset copies, not per-row work; no new scan, RPC, retry or significant allocation. Test iterator/column/predicate ownership is sound. |
| Other module obligations | No hub header, PCH, extern-template, unity-skip, hygiene-gate or libc/link dependency change. Physical IDs are not confused with segment positions. No other substantiated issue remains. |
Relevant flow:
```text
visible rowset -> index spec -> manual_build -> same-version replace -> production save_meta
local visible/stale -> rename/build transient rowset -> output PB + TSO -> persisted header
Cloud source -> fresh output PB + TSO -> existing PB conversion -> restore metadata persistence
```
### Verification boundaries
This review was read-only: no build, test execution, source modification or GitHub publication. Existing test evidence is from the implementation phase. Index UT does not execute production save_meta under BE_TEST, so it is not a restart-persistence test. Local snapshot UT uses empty rowsets to check visible/stale header rewriting. Cloud UT verifies metadata conversion, not actual S3/MS RPC or Cloud SQL RESTORE; active Row Binlog source/target restore is rejected by FE. The patch prevents future metadata omissions, not recovery of already-lost historical timestamps.
<!-- doris-repo-review:v1:begin -->
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#68793
commit: 6fe5c44c417c71c5d82ae4bc22bd6ae90032e785
base: 038e2274ef678c44f85214e31da6d3e1d48a794f
reviewed_at: 2026-10-09T06:32:34+00:00
reviewer: luwei16
model: gpt-6.1-sol
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 2
converged: true
```
<!-- doris-repo-review:v1:end -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6063225423
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://doris.apache.org/community/how-to-contribute/review-and-ci).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](binlog) Preserve commit TSO across rowset metadata rewrites [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #68793:
URL: https://github.com/apache/doris/pull/68793#issuecomment-6066936049
# BE Regression && UT Coverage Report
Increment line coverage `100% (0/0)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/68793_6fe5c44c417c71c5d82ae4bc22bd6ae90032e785_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/68793_6fe5c44c417c71c5d82ae4bc22bd6ae90032e785_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 76.68% (35189/45892) |
| Line Coverage | 61.86% (397528/642607) |
| Region Coverage | 58.39% (336030/575477) |
| Branch Coverage | 59.23% (154445/260753) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org