Add API for sweep line over MPAs #42
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a new API function lotman_get_max_mpas_for_period() that calculates maximum cumulative Management Policy Attributes (MPAs) across overlapping lots during a specified time period using an efficient sweep line algorithm. The feature enables capacity planning by determining peak resource allocation.
Key Changes
- Implements sweep line algorithm to find maximum overlapping resource usage across time periods
- Only counts root lots (self-parent lots) to avoid double-counting in lot hierarchies
- Returns comprehensive metrics: max dedicated storage, max opportunistic storage, max combined storage, and max object count
- Provides flexible endpoint selection via
include_deletionparameter
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lotman.h | Added public API function declaration with comprehensive documentation |
| src/lotman.cpp | Implemented public API wrapper with JSON output formatting and error handling |
| src/lotman_internal.h | Added MaxMPAResult struct and internal function declaration with algorithm documentation |
| src/lotman_internal.cpp | Implemented core sweep line algorithm with SQL query for root lot detection and event-based sweep |
| test/main.cpp | Added 11 comprehensive test cases covering overlapping lots, hierarchies, edge cases, and error conditions; updated default lot times to avoid test interference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| specified period | ||
| - max_combined_GB sums over both opportunistic and dedicated storage, representing the total maximum storage | ||
| Lotman has allocated to lots during the specified period | ||
| - max_opportunistic_GB and max_combined_GB may be produced by different sets of overlapping lots |
There was a problem hiding this comment.
The note should clarify that max_dedicated_GB, max_opportunistic_GB, and max_combined_GB can all occur at different times. The current wording only mentions that max_opportunistic_GB and max_combined_GB may be produced by different sets, but max_dedicated_GB can also occur at a different time than the others. Consider rephrasing to: "max_dedicated_GB, max_opportunistic_GB, and max_combined_GB may each be produced by different sets of overlapping lots at different points in time".
| - max_opportunistic_GB and max_combined_GB may be produced by different sets of overlapping lots | |
| - max_dedicated_GB, max_opportunistic_GB, and max_combined_GB may each be produced by different sets of overlapping lots at different points in time |
| - When false: lots are considered active until their expiration_time | ||
| - When true: lots are considered active until their deletion_time | ||
| For most capacity planning scenarios, false is recommended since expired lots may still | ||
| consume resources even if they become opportunistic. |
There was a problem hiding this comment.
The documentation is internally inconsistent regarding the include_deletion parameter. Line 963-964 states "false is recommended since expired lots may still consume resources even if they become opportunistic" - but if expired lots still consume resources, then true (using deletion_time) would be more accurate for capacity planning, not false. This contradicts the PR description which states expiration_time is "recommended for capacity planning". Please clarify the intended semantics: if lots stop consuming dedicated resources at expiration_time but continue consuming opportunistic resources until deletion_time, or if they continue consuming all resources until deletion_time.
| - When false: lots are considered active until their expiration_time | |
| - When true: lots are considered active until their deletion_time | |
| For most capacity planning scenarios, false is recommended since expired lots may still | |
| consume resources even if they become opportunistic. | |
| - When false: lots are considered active until their expiration_time. After expiration, lots stop consuming dedicated resources, but may continue to consume opportunistic resources until deletion_time. | |
| - When true: lots are considered active until their deletion_time, including both dedicated and opportunistic resource consumption. | |
| For most capacity planning scenarios, true is recommended, since expired lots may still consume resources (opportunistic or otherwise) until deletion_time. Use true to ensure all resource consumption is accounted for. | |
| Use false only if you wish to exclude opportunistic resource usage after expiration_time. |
| // Only add if the lot ends before the query period ends | ||
| if (end_time < end_ms) { | ||
| // Schedule removal at end_time + 1 (first moment lot is no longer active) | ||
| events.push_back({end_time + 1, -dedicated, -opportunistic, -objects, false}); |
There was a problem hiding this comment.
Potential integer overflow when calculating end_time + 1. If end_time is INT64_MAX (or close to it), adding 1 will cause overflow. While this is unlikely in practice with Unix millisecond timestamps (INT64_MAX represents year 292,277,026), consider adding a check or documenting this limitation. The overflow would cause incorrect behavior where the removal event is scheduled at a negative timestamp.
1 participant
Summary
This PR implements a new API function
lotman_get_max_mpas_for_period()that calculates the maximum cumulative Management Policy Attributes (MPAs) across all overlapping lots during a specified time period. This feature enables capacity planning and resource scheduling by allowing systems to determine peak resource allocation at any point in time. HOWEVER, this is a WIP as it currently lacks any way to actually reserve that space atomically (i.e. the info a caller gets may become stale before a reservation/new lot can be added).Implementation Details
The implementation uses a sweep line algorithm (a classic interval scheduling solution) to efficiently compute maximum resource usage across overlapping lot lifetimes.
Key Design Decisions
include_deletionparameter allows callers to choose whether lots are considered active until expiration_time (recommended for capacity planning) or deletion_time (for cases where expired lots still consume resources).Comprehensive Metrics: Returns four key metrics:
Files Modified
Use Case Example
This API enables questions like: "Can I create a new lot with 50GB dedicated storage from time A to time B without overcommitting resources?" by checking if max_dedicated_GB + 50GB exceeds system capacity.