-
Notifications
You must be signed in to change notification settings - Fork 525
Documenting memory usage/management by BatchLogProcessor #2469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
6c84650
1b5f52b
76973f4
67a1b32
ba36c8f
403a412
1282bfc
24797bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -167,6 +167,72 @@ enum BatchMessage { | |
|
||
/// A [`LogProcessor`] that buffers log records and reports | ||
/// them at a pre-configured interval from a dedicated background thread. | ||
// **Memory Management in BatchLogProcessor** | ||
// | ||
// The `BatchLogProcessor` manages memory through the following stages of log processing: | ||
// | ||
// 1. **Record Ingestion**: | ||
// - Each `LogRecord` is **cloned** upon entering the processor. | ||
// - `LogRecordAttributes` utilize a hybrid memory model: | ||
// - Attributes up to `PREALLOCATED_ATTRIBUTE_CAPACITY` are **stack-allocated**. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since - The first 5 attributes are allocated on the stack.
- Any additional attributes are allocated on the heap in a dynamically growing vector. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// - Exceeding attributes trigger **heap allocation** in a dynamically growing vector. | ||
// - The `LogRecord` and its associated `InstrumentationScope` are **boxed together** | ||
// to allocate them on the heap before entering the queue. Which means: | ||
// - The entire `LogRecord` object, including its stack-allocated inline attributes, is moved to the heap. | ||
// - Any overflow attributes already on the heap remain unaffected. | ||
// | ||
// 2. **Queue Management**: | ||
// - Uses a **bounded synchronous channel** (`sync_channel`) with a maximum size defined by `max_queue_size`. | ||
// - Each queued item is a **heap-allocated** `Box<(LogRecord, InstrumentationScope)>`. | ||
// - When the queue is full: | ||
// - Records are **dropped** (counted but not stored). | ||
// - A warning is logged the first time this occurs. | ||
// | ||
// 3. **Worker Thread Storage**: | ||
// - The worker thread maintains a pre-allocated `Vec` of boxed record pairs: | ||
// - The vector’s capacity is fixed at `max_export_batch_size`. | ||
// - Records are **moved** (not cloned) from the queue to the vector for processing. | ||
// | ||
// 4. **Export Process**: | ||
// - A temporary `Vec` is created during the export process to hold ownership of the boxed records: | ||
// - The contents of the worker thread's buffer (`Vec<Box<(LogRecord, InstrumentationScope)>>`) | ||
// are moved into this temporary vector using `logs.split_off(0)`. | ||
utpilla marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// - Ownership of the boxed records is transferred to the new vector, ensuring efficient | ||
// memory usage without additional cloning. | ||
// - The temporary vector is then used to construct references passed to the exporter via `LogBatch`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be good to mention that there two temporary vectors created during each export:
I think we should be able to avoid the second vector creation as well by updating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created #2488 to showcase one approach to avoid the vec allocation mentioned in the second point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have updated the document as per the latest optimizations, where both these vectors usage is removed. |
||
// - The exporter's `export()` method receives references to the log records and `InstrumentationScope` | ||
// contained in `LogBatch`. | ||
// - If the exporter requires retaining the log records (e.g., for retries or asynchronous | ||
// operations), it must **clone** the records inside the `export()` implementation. | ||
// - After successful export: | ||
// - The original boxed records are dropped, releasing heap memory. | ||
// - Export is triggered in the following scenarios: | ||
// - When the batch size reaches `max_export_batch_size`. | ||
// - When the scheduled delay timer expires. | ||
// - When `force_flush` is called. | ||
// - During processor shutdown. | ||
// | ||
/// 5. **Memory Limits**: | ||
/// - **Worst-Case Memory Usage**: | ||
/// - **Queue Memory** = `max_queue_size * size of boxed (LogRecord + InstrumentationScope)`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd be nice to mention the actual size of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, have added some numbers of memory usage in worst case scenario, with average size of LogRecord and InstrumentationScope. |
||
/// - **Batch Memory** = `max_export_batch_size * size of boxed (LogRecord + InstrumentationScope)`. | ||
/// - **Total Maximum Memory:** | ||
/// - When both the queue and batch vector are full: | ||
/// ``` | ||
/// (max_queue_size + max_export_batch_size) * size of boxed (LogRecord + InstrumentationScope) | ||
/// ``` | ||
/// - The temporary vector used during export only holds references to records, | ||
/// so it does not contribute additional significant memory allocation. | ||
// | ||
// 6. **Key Notes on Memory Behavior**: | ||
// - Boxing a `LogRecord` and `InstrumentationScope` moves the record to the heap, | ||
// including stack-allocated attributes. | ||
// - During the export process, ownership of the boxed records is transferred | ||
// to a temporary vector created by `logs.split_off(0)`. | ||
// - This temporary vector is then used to construct references passed to the exporter. | ||
// - No additional cloning or copying occurs during the export process, minimizing | ||
// memory overhead while ensuring efficient handling of log records. | ||
// | ||
pub struct BatchLogProcessor { | ||
message_sender: SyncSender<BatchMessage>, | ||
handle: Mutex<Option<thread::JoinHandle<()>>>, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a lot of these can be moved off to docs.rs itself, as they are good for users to be aware of.