[exporterhelper] Support mixed sizer types in persistent queue without draining #13246
Conversation
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
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (77.92%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #13246 +/- ##
==========================================
- Coverage 91.57% 91.45% -0.12%
==========================================
Files 522 521 -1
Lines 29089 29155 +66
==========================================
+ Hits 26639 26665 +26
- Misses 1933 1959 +26
- Partials 517 531 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was referenced Jun 22, 2025
malus2077
changed the title
This refactoring eliminates sizer type dependencies in persistent queues, enabling seamless mixed sizer usage without requiring queue draining. Key changes: - Embed itemSize in each QueueItem for mixed sizer support - Consolidate metadata storage with single protobuf structure (qmv0 key) - Add backward compatibility with automatic legacy format migration - Remove obsolete SizerType enum and conversion logic - Update persistent queue logic for new metadata format - Add comprehensive migration tests and update existing tests - Add changelog entry for enhancement tracking Fixes open-telemetry#12890 Related to: open-telemetry#13126
malus2077
force-pushed
the
sizer-drain-optimization-12890
branch
from
abf1753 to
70024af
Compare
bogdandrutu
reviewed
Jun 23, 2025
Comment on lines
+22
to
+27
| // QueueItem represents a single item stored in the persistent queue. | ||
| message QueueItem { | ||
| // Size of the item according to the sizer that was active when enqueued | ||
| int64 item_size = 1; | ||
| // The marshaled data of the item. | ||
| bytes item_data = 2; |
There was a problem hiding this comment.
We did a lot of work to avoid multiple copies and allocations of the marshalled bytes in @dmitryax pdatax work, and here we will just re-allocate and copy the whole data.
There was a problem hiding this comment.
Should the current PR be closed?
bogdandrutu
requested changes
Jun 23, 2025
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.
Summary
Implements non-blocking queue handling for sizer configuration changes by embedding item size information directly into queue items, eliminating the need to drain queues when switching between different sizer types.
Problem Statement
Solution
itemSizein each queue item using newQueueItemprotobuf messageqmv0keySizerTypetrackingBreaking Changes
None - Fully backward compatible with automatic migration
Testing
Performance Impact