[BUG](frontend): Meter IDs-only deletes - #7657
Open
Jokasa7 wants to merge 1 commit into
Open
Conversation
Close the read metering context after delete selector paths converge so IDs-only requests submit the existing billing event. Add regression coverage that exercises a metered IDs-only delete.
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes
CollectionReadContextafter the delete selector branches haveconverged, instead of closing it only after a
wherelookup.CollectionRead(GetForDelete)event for IDs-only andempty-selector delete paths as well as
where-based deletes.wherebranch and theexisting
metering_events_sent.readsuccess counter.a real metering context and verifies that exactly one read event is emitted
with the expected action and region.
changing the delete API or record-selection behavior.
Closes #6695.
Test plan
cargo testfor Rust.CollectionReadevents where one was expected.cargo test -p chroma-frontend --test test_delete_limit test_delete_by_ids_emits_collection_read_metering_event -- --exact --nocapturepasses after the fix.cargo test -p chroma-frontend --test test_delete_limit -- --nocapturepasses all 8 tests.
cargo clippy -p chroma-frontend --test test_delete_limit -- -D warningspasses.
cargo fmt --all --checkandgit diff --checkpass.Migration plan
No migration or compatibility step is required. The change only ensures that
an already-created read metering context reaches the existing event receiver on
every delete-selector path.
Observability plan
No new metric is introduced. Successful submission continues to increment the
existing
metering_events_sent.readcounter, and the emitted event retains theexisting
GetForDeleteaction and region fields. That counter and the billingevent stream can be used to confirm that IDs-only deletes are no longer absent.
Documentation Changes
No user-facing API, configuration, or behavior documentation changes are
needed. This is an internal metering correctness fix with regression coverage.