You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "yx-keith (via GitHub)" <gi...@apache.org> on 2026/05/30 16:17:32 UTC
[PR] [improvement](be) Train ANN index once instead of on every chunk [doris]
yx-keith opened a new pull request, #63913:
URL: https://github.com/apache/doris/pull/63913
AnnIndexColumnWriter buffers vectors and flushes a full chunk (ann_index_build_chunk_size, default 1,000,000 rows) at a time, calling train() then add() on each flush. Re-invoking train() per chunk is wrong:
- For IVF with a SQ/PQ quantizer, train() re-fits the scalar/product codebook on the latest chunk (FAISS IndexIVF::train runs train_encoder with no is_trained guard). Vectors from earlier chunks were already add()ed and encoded under the previous codebook, so after the final train() their stored codes no longer match the codebook used at search time, corrupting recall on any segment larger than one chunk.
- For the default IVFFlat the coarse-quantizer k-means is guarded (Level1Quantizer::train_q1 returns early once quantizer->ntotal == nlist), so recall is not corrupted, but every chunk still pays a redundant train() call (and its OpenMP build-budget mutex).
The writer's own finish() comment already documents the intended contract ("the quantizer was already trained on previous batches, just add"), but the per-chunk loop never honored it.
Train at most once per index lifetime:
- Add VectorIndex::needs_training(); FaissVectorIndex returns true for IVF / IVF_ON_DISK and for any non-FLAT quantizer, false for HNSW+FLAT.
- Route both train() call sites through _train_once_if_needed(), guarded by a _trained flag, so train() runs at most once and HNSW+FLAT skips it entirely.
Tests:
- MockVectorIndex gains a needs_training() mock; the IVF/PQ/SQ writer tests now assert train() is invoked exactly once rather than per chunk.
- New TestHnswSkipsTrainCall pushes multiple full chunks through an index that reports needs_training()==false and asserts zero train() calls.
- New NeedsTrainingMatrix unit-tests FaissVectorIndex::needs_training() across index types and quantizers.
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] 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? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] 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 -->
--
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] [improvement](be) Train ANN index once instead of on every chunk [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63913:
URL: https://github.com/apache/doris/pull/63913#issuecomment-4583399307
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
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] [improvement](be) Train ANN index once instead of on every chunk [doris]
Posted by "yx-keith (via GitHub)" <gi...@apache.org>.
yx-keith closed pull request #63913: [improvement](be) Train ANN index once instead of on every chunk
URL: https://github.com/apache/doris/pull/63913
--
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] [improvement](be) Train ANN index once instead of on every chunk [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63913:
URL: https://github.com/apache/doris/pull/63913#issuecomment-4585936724
<details>
<summary>TPC-DS: <b>Total hot run time: 171456 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 d06433e0521e7919580f64b9635dcd81197fac17, data reload: false
query5 4309 700 523 523
query6 324 225 203 203
query7 4215 554 330 330
query8 319 230 222 222
query9 8805 4042 4061 4042
query10 478 354 306 306
query11 5841 2412 2237 2237
query12 189 131 128 128
query13 1299 581 463 463
query14 6130 5510 5158 5158
query14_1 4462 4499 4455 4455
query15 219 209 188 188
query16 1029 464 446 446
query17 973 740 588 588
query18 2458 486 351 351
query19 218 197 154 154
query20 139 145 138 138
query21 223 135 118 118
query22 13722 13602 13335 13335
query23 17455 16714 16155 16155
query23_1 16406 16428 16319 16319
query24 7532 1791 1345 1345
query24_1 1316 1327 1340 1327
query25 552 486 418 418
query26 1315 327 174 174
query27 2676 526 356 356
query28 4431 1987 2010 1987
query29 954 651 493 493
query30 302 240 202 202
query31 1144 1073 994 994
query32 93 75 72 72
query33 530 355 291 291
query34 1200 1156 641 641
query35 792 803 728 728
query36 1431 1407 1258 1258
query37 159 114 95 95
query38 3241 3167 3084 3084
query39 941 917 908 908
query39_1 882 886 884 884
query40 236 153 129 129
query41 73 70 68 68
query42 115 111 110 110
query43 337 346 294 294
query44
query45 216 202 196 196
query46 1072 1181 749 749
query47 2471 2343 2273 2273
query48 412 439 315 315
query49 652 515 409 409
query50 1064 346 256 256
query51 4369 4279 4212 4212
query52 107 108 94 94
query53 275 278 208 208
query54 329 290 269 269
query55 102 93 88 88
query56 305 338 326 326
query57 1446 1447 1327 1327
query58 319 290 282 282
query59 1626 1647 1450 1450
query60 340 346 339 339
query61 186 179 182 179
query62 708 664 583 583
query63 282 200 211 200
query64 2383 809 637 637
query65
query66 1738 476 355 355
query67 29867 29862 29011 29011
query68
query69 474 370 305 305
query70 1020 997 1035 997
query71 315 274 265 265
query72 3057 2741 2410 2410
query73 868 788 412 412
query74 5082 5000 4798 4798
query75 2682 2635 2262 2262
query76 2266 1143 787 787
query77 423 406 335 335
query78 12497 12443 11924 11924
query79 1505 1062 762 762
query80 665 538 453 453
query81 447 286 241 241
query82 1442 157 120 120
query83 361 278 253 253
query84 311 140 112 112
query85 891 559 462 462
query86 390 328 353 328
query87 3434 3388 3213 3213
query88 3606 2726 2725 2725
query89 448 398 353 353
query90 1893 182 179 179
query91 189 174 147 147
query92 80 78 75 75
query93 1451 1542 855 855
query94 551 344 310 310
query95 690 391 356 356
query96 1027 772 327 327
query97 2721 2722 2666 2666
query98 237 233 233 233
query99 1182 1162 1026 1026
Total cold run time: 255130 ms
Total hot run time: 171456 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] [improvement](be) Train ANN index once instead of on every chunk [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63913:
URL: https://github.com/apache/doris/pull/63913#issuecomment-4585912315
<details>
<summary>TPC-H: <b>Total hot run time: 32140 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 d06433e0521e7919580f64b9635dcd81197fac17, data reload: false
------ Round 1 ----------------------------------
orders Doris NULL NULL 0 0 0 NULL 0 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 17600 4060 4025 4025
q2 q3 10761 1469 859 859
q4 4684 477 340 340
q5 7546 2322 2129 2129
q6 251 190 143 143
q7 947 810 626 626
q8 9490 1727 1665 1665
q9 5165 4996 4995 4995
q10 6397 2240 1891 1891
q11 435 274 240 240
q12 626 436 294 294
q13 18163 3455 2822 2822
q14 269 255 231 231
q15 q16 821 774 708 708
q17 924 947 991 947
q18 6988 5776 5630 5630
q19 1227 1269 1209 1209
q20 577 459 318 318
q21 5983 2878 2742 2742
q22 524 375 326 326
Total cold run time: 99378 ms
Total hot run time: 32140 ms
----- Round 2, with runtime_filter_mode=off -----
orders Doris NULL NULL 150000000 42 6422171781 NULL 22778155 NULL NULL 2023-12-26 18:27:23 2023-12-26 18:42:55 NULL utf-8 NULL NULL
============================================
q1 4874 4941 4829 4829
q2 q3 5000 5282 4667 4667
q4 2125 2203 1444 1444
q5 5017 4716 4713 4713
q6 235 182 129 129
q7 1887 1714 1588 1588
q8 2438 2151 2111 2111
q9 7708 7460 7394 7394
q10 4832 4683 4247 4247
q11 541 387 353 353
q12 733 737 532 532
q13 3017 3395 2807 2807
q14 291 291 251 251
q15 q16 685 709 622 622
q17 1296 1258 1264 1258
q18 7273 6873 6906 6873
q19 1164 1123 1123 1123
q20 2211 2262 1956 1956
q21 5304 4629 4510 4510
q22 525 476 425 425
Total cold run time: 57156 ms
Total hot run time: 51832 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] [improvement](be) Train ANN index once instead of on every chunk [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63913:
URL: https://github.com/apache/doris/pull/63913#issuecomment-4586199369
# BE Regression && UT Coverage Report
Increment line coverage `95.00% (19/20)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/63913_d06433e0521e7919580f64b9635dcd81197fac17_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/63913_d06433e0521e7919580f64b9635dcd81197fac17_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.81% (28135/38120) |
| Line Coverage | 57.74% (305496/529130) |
| Region Coverage | 54.87% (255403/465508) |
| Branch Coverage | 56.41% (110411/195732) |
--
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] [improvement](be) Train ANN index once instead of on every chunk [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #63913:
URL: https://github.com/apache/doris/pull/63913#issuecomment-4586061595
# BE UT Coverage Report
Increment line coverage `95.00% (19/20)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/d06433e0521e7919580f64b9635dcd81197fac17_d06433e0521e7919580f64b9635dcd81197fac17/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/d06433e0521e7919580f64b9635dcd81197fac17_d06433e0521e7919580f64b9635dcd81197fac17/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 54.01% (21027/38931) |
| Line Coverage | 37.55% (199207/530508) |
| Region Coverage | 33.81% (155919/461108) |
| Branch Coverage | 34.80% (67857/195006) |
--
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] [improvement](be) Train ANN index once instead of on every chunk [doris]
Posted by "yx-keith (via GitHub)" <gi...@apache.org>.
yx-keith commented on PR #63913:
URL: https://github.com/apache/doris/pull/63913#issuecomment-4585755527
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