The vacuum lock was a solution to preventing race conditions during a trailing transaction cleanup, so that only one closed txn could be pruning the tail at a time.
However, there is a better solution. Instead, each txn on commit increments it's "next" commit. IE when we commit, the ref count is set to 2. One for active, one because the previous txn has a reference.
Then when the previous transaction is closed, it decrements the ref count of the next txn. If the txn is active, this would drop to 1.
If the txn was a middle txn, IE
A --> B --> C
When A drops to 0, it would drop B to 0 as well, causing both to be cleaned - Note that if B was held as a RO txn to a thread, the ref count would still be positive.
The benefit to this is that due to the use of the atomics in the ref count, only the last holder is guaranteed to do the tail clean up.
A consideration is this must be iterative not recursive due to the potential for a long transaction history to be cleaned, and we don't want to blow past the end of the stack.
Metadata Update from @firstyear: - Custom field type adjusted to defect - Issue assigned to firstyear - Issue priority set to: 3 - Issue tagged with: Complex, Performance, SDS
<img alt="0001-Ticket-49153-Remove-vacuum-lock-on-transaction-clean.patch" src="/389-ds-base/issue/raw/files/caa388aaee538a194605ae490b03e192d3df4bb276a676e5a6c72a224d1b3e52-0001-Ticket-49153-Remove-vacuum-lock-on-transaction-clean.patch" />
Metadata Update from @firstyear: - Custom field reviewstatus adjusted to new
Benchmark of this improvement on my laptop. This is a significant improvement over the vacuum lock (in some cases by 10 times over the previous strategy), and much faster than the pthread rwlock.
Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: pl hashmap large bench_insert_search_delete_batch 5000 times tid 0: 2.0366751749 tid 1: 2.0261795191 tid 2: 2.0331016160 tid 3: 2.1262398761 tid 4: 2.1355573617 tid 5: 3.1434805328 tid 6: 3.0335149732 tid 7: 3.1464685231 tid 8: 3.1457265706 tid 9: 3.1433246679 Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: sds b+tree without search csum bench_insert_search_delete_batch 5000 times tid 0: 2.0833829650 tid 1: 3.0059421377 tid 2: 3.0029089924 tid 3: 3.0286917908 tid 4: 3.0399154019 tid 5: 5.0241727446 tid 6: 5.0230666762 tid 7: 4.0645493347 tid 8: 5.0070896634 tid 9: 5.0240911266 Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: sds b+tree with copy on write bench_insert_search_delete_batch 5000 times tid 0: 3.0417506591 tid 1: 3.0394962227 tid 2: 3.0364499744 tid 3: 2.1112221629 tid 4: 2.1063986565 tid 5: 1.1125640334 tid 6: 1.1225960708 tid 7: 1.1173347458 tid 8: 1.1133565439 tid 9: 1.1143226880 --- Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: pl hashmap large bench_isd_write_delay 5000 times tid 0: 43.0159074010 tid 1: 43.1673444145 tid 2: 33.1355757152 tid 3: 43.1272677308 tid 4: 43.1384976384 tid 5: 43.1163829944 tid 6: 43.1327795467 tid 7: 43.1302892597 tid 8: 43.1186886169 tid 9: 43.1167317529 Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: sds b+tree without search csum bench_isd_write_delay 5000 times tid 0: 42.1008799159 tid 1: 41.0329086244 tid 2: 42.1320641988 tid 3: 46.0146168811 tid 4: 46.1539020150 tid 5: 46.0204698455 tid 6: 46.1448575564 tid 7: 46.1640176107 tid 8: 46.0321916972 tid 9: 46.1472235845 Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: sds b+tree with copy on write bench_isd_write_delay 5000 times tid 0: 35.1020421658 tid 1: 43.0710900632 tid 2: 40.1153746476 tid 3: 1.1064356510 tid 4: 1.1052480255 tid 5: 1.1108705059 tid 6: 1.1047440924 tid 7: 1.1106902055 tid 8: 1.1059089301 tid 9: 1.1072875233 --- Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: pl hashmap large bench_isd_read_delay 5000 times tid 0: 196.1681798306 tid 1: 196.1669011902 tid 2: 23.1361075130 tid 3: 193.1354246221 tid 4: 193.1642176140 tid 5: 192.1057488894 tid 6: 195.1898362053 tid 7: 190.1876505439 tid 8: 184.1424917689 tid 9: 196.0035411707 Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: sds b+tree without search csum bench_isd_read_delay 5000 times tid 0: 196.0618862574 tid 1: 196.0646437178 tid 2: 196.0578889155 tid 3: 183.0130031216 tid 4: 191.0706960608 tid 5: 196.0143631961 tid 6: 195.0713675603 tid 7: 189.0254152826 tid 8: 193.1127186233 tid 9: 189.0079376099 Added 4096 BENCH: Start ... BENCH: Complete ... BENCH: sds b+tree with copy on write bench_isd_read_delay 5000 times tid 0: 1.1790740024 tid 1: 1.1656894180 tid 2: 1.1662494341 tid 3: 24.0002030525 tid 4: 24.0022017272 tid 5: 23.1019467589 tid 6: 24.1802803761 tid 7: 24.1876700166 tid 8: 24.0020759954 tid 9: 24.1869703177 ---
Metadata Update from @firstyear: - Custom field reviewstatus adjusted to review (was: new)
Metadata Update from @mreynolds: - Custom field reviewstatus adjusted to ack (was: review)
commit 13f202f To ssh://git@pagure.io/389-ds-base.git fcfe4e4..0550cea master -> master
Metadata Update from @firstyear: - Issue close_status updated to: fixed - Issue status updated to: Closed (was: Open)
389-ds-base is moving from Pagure to Github. This means that new issues and pull requests will be accepted only in 389-ds-base's github repository.
This issue has been cloned to Github and is available here: - https://github.com/389ds/389-ds-base/issues/2212
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: fixed)
Login to comment on this ticket.