Skip to content

Always Copy-on-Write when updating payload in immutable segments#7952

Merged
ffuugoo merged 4 commits intodevfrom
cow-payload-update
Jan 21, 2026
Merged

Always Copy-on-Write when updating payload in immutable segments#7952
ffuugoo merged 4 commits intodevfrom
cow-payload-update

Conversation

@ffuugoo
Copy link
Contributor

@ffuugoo ffuugoo commented Jan 20, 2026

This makes partial snapshots more efficient. Without this change, if we modify a single payload (key) in immutable segment, the whole payload storage (for this segment) have to be included into the snapshot.

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you formatted your code locally using cargo +nightly fmt --all command prior to submission?
  3. Have you checked your code using cargo clippy --workspace --all-features command?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@ffuugoo ffuugoo added this to the Qdrant on Edge milestone Jan 20, 2026
@ffuugoo ffuugoo requested review from generall and timvisee January 20, 2026 14:14
@qdrant qdrant deleted a comment from coderabbitai bot Jan 20, 2026
Copy link
Member

@timvisee timvisee left a comment

Choose a reason for hiding this comment

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

Yes, that's it.

Though if we go all in, we can probably remove that update_nonappendable closure all together to reduce complexity.

If this turns out to be a bad change in the future, we can revert this PR to regain the functionality.

@agourlay
Copy link
Member

What is the rational for this change exactly?

@ffuugoo
Copy link
Contributor Author

ffuugoo commented Jan 20, 2026

What is the rational for this change exactly?

Partial snapshots don't work well enough if we modify payload in-place in immutable segments.

@timvisee
Copy link
Member

What is the rational for this change exactly?

Make immutable segments properly immutable.

It has two benefits:

  1. simpler R/W synchronization (partial snapshots) implementation, we now don't have to track changes in immutable segments
  2. make immutable segments lock free (hopefully aids performance)

There is one exception that remains: deleted flags. But it's likely we can implement this in a lock free fashion with atomics.

Our guess is that impact on users is minimal to nothing. For existing users this would only have been beneficial if they'd update payload that also is not indexed. We believe that doesn't really happen often in practice.

@coderabbitai

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@generall
Copy link
Member

@ffuugoo please resolve conflict

@ffuugoo
Copy link
Contributor Author

ffuugoo commented Jan 20, 2026

@ffuugoo please resolve conflict

Who did this!? 😡

@ffuugoo ffuugoo force-pushed the cow-payload-update branch from ae81a34 to 85e1c99 Compare January 20, 2026 20:01
@ffuugoo
Copy link
Contributor Author

ffuugoo commented Jan 21, 2026

@ffuugoo please resolve conflict

Rebased

@ffuugoo ffuugoo merged commit 7bc947c into dev Jan 21, 2026
15 checks passed
@ffuugoo ffuugoo deleted the cow-payload-update branch January 21, 2026 10:09
@timvisee timvisee mentioned this pull request Feb 17, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants