Skip to content

Conversation

@markuspf
Copy link
Contributor

@markuspf markuspf commented Dec 10, 2025

Scope & Purpose

Introduces thread-local leasers for velocypack::Builder and std::string.

This also addresses a problem with async-prefetch where builders and strings were leased through the transaction context, which is not meant to be acceessed concurrently.

  • 💩 Bugfix
  • 🍕 New feature
  • 🔥 Performance improvement
  • 🔨 Refactoring/simplification

Fixes #22158 and other issues with async-prefetch


Note

Introduce thread-local leasers for Builder and std::string and refactor transaction leasers to use them, fixing async-prefetch concurrency issues and adding targeted tests.

  • Basics:
    • New ThreadLocalLeaser: Add Basics/ThreadLocalLeaser.h with thread-local pooling for reusable objects; typedefs ThreadLocalBuilderLeaser and ThreadLocalStringLeaser.
    • Tests: Add ThreadLocalLeaserTest.cpp covering leasing, stash behavior, moves, and cross-thread handoff.
  • Transaction:
    • Refactor leasers: transaction::StringLeaser and transaction::BuilderLeaser now use ThreadLocal*Leaser (release/acquire/get); remove context-based leasing/destructors/clear/steal.
    • Integrations: Update usages in Helpers.cpp/.h, Methods.cpp (drop explicit _replicationData.clear()), and helpers to new APIs.
  • Mocks/Tests:
    • Update tests/Mocks/PhysicalCollectionMock.cpp to use builder.release().
    • Add JS regression test aql-async-prefetch-leaser-reuse.js to stress async-prefetch concurrency.
  • Changelog:
    • Note fix for concurrent reuse of builders/strings with async-prefetch.

Written by Cursor Bugbot for commit c0d5590. This will update automatically on new commits. Configure here.

@cla-bot cla-bot bot added the cla-signed label Dec 10, 2025
@markuspf
Copy link
Contributor Author

Should add a test that at least tickles TSAN when run.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on December 28

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@markuspf markuspf changed the title Introduce thread-local leasers for builders and strings [COR-50] Introduce thread-local leasers for builders and strings Dec 11, 2025
@markuspf
Copy link
Contributor Author

Should also measure performance implications.

At the moment there is only a very small number of leasable builders/string available per thread, as opposed to the previously unbounded number per transaction.

@markuspf markuspf force-pushed the bug-fix/thread-local-leasers branch from a4e360a to 46a60a2 Compare December 12, 2025 12:00
@markuspf markuspf force-pushed the bug-fix/thread-local-leasers branch from 371ba8d to 89a2ec8 Compare December 15, 2025 11:25
@markuspf markuspf force-pushed the bug-fix/thread-local-leasers branch from a1709b0 to 4278066 Compare December 15, 2025 16:01
@markuspf
Copy link
Contributor Author

For the record: simple performance tests did not show any effect of this change on performance.

@markuspf markuspf merged commit 08f3b31 into devel Dec 16, 2025
9 checks passed
@markuspf markuspf deleted the bug-fix/thread-local-leasers branch December 16, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ArangoDB 3.12.5 When Running Specific Query AQL-Builder Error is returned, crashing eventually

2 participants