-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add evacuate_limit_size to bound chunk evacuation #5473
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -72,6 +72,9 @@ class BufferChunkOverflowError < BufferError; end # A record size is larger than | |
| desc 'If true, chunks are thrown away when unrecoverable error happens' | ||
| config_param :disable_chunk_backup, :bool, default: false | ||
|
|
||
| desc 'The total size limit for chunks evacuated on unrecoverable errors. Once evacuation reaches this size, remaining chunks are purged without being evacuated. Set 0 to disable evacuation. nil (default) means no limit.' | ||
| config_param :evacuate_limit_size, :size, default: nil | ||
|
|
||
| Metadata = Struct.new(:timekey, :tag, :variables, :seq) do | ||
| def initialize(timekey, tag, variables) | ||
| super(timekey, tag, variables, 0) | ||
|
|
@@ -625,11 +628,15 @@ def clear_queue! | |
| log.on_trace { log.trace "clearing queue", instance: self.object_id } | ||
|
|
||
| synchronize do | ||
| evacuated_size = 0 | ||
| until @queue.empty? | ||
| begin | ||
| q = @queue.shift | ||
| evacuate_chunk(q) | ||
| log.trace("purging a chunk in queue"){ {id: dump_unique_id_hex(chunk.unique_id), bytesize: chunk.bytesize, size: chunk.size} } | ||
| if evacuate_chunk?(q.bytesize, evacuated_size) | ||
| evacuate_chunk(q) | ||
| evacuated_size += q.bytesize | ||
|
Comment on lines
+636
to
+637
Contributor
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. In Worth confirming success (or having |
||
| end | ||
|
Comment on lines
+635
to
+638
|
||
| log.trace("purging a chunk in queue"){ {id: dump_unique_id_hex(q.unique_id), bytesize: q.bytesize, size: q.size} } | ||
| q.purge | ||
| rescue => e | ||
| log.error "unexpected error while clearing buffer queue", error_class: e.class, error: e | ||
|
|
@@ -640,6 +647,17 @@ def clear_queue! | |
| end | ||
| end | ||
|
|
||
| # Decide whether a chunk should be evacuated, honoring evacuate_limit_size. | ||
| # nil limit means no bound (evacuate everything), 0 disables evacuation, and | ||
| # a positive limit only evacuates a chunk if it fits within the remaining budget | ||
| # so the total evacuated size never exceeds the limit. | ||
| def evacuate_chunk?(chunk_bytesize, evacuated_size) | ||
| return true if @evacuate_limit_size.nil? | ||
| return false if @evacuate_limit_size <= 0 | ||
|
|
||
| evacuated_size + chunk_bytesize <= @evacuate_limit_size | ||
| end | ||
|
|
||
| def evacuate_chunk(chunk) | ||
| # Overwrite this on demand. | ||
| # | ||
|
|
||
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.
[Nit] since the budget actually resets on every
clear_queue!call rather than being a true cumulative cap, the nameevacuate_limit_sizeand the desc's "total size limit" phrasing read as more absolute than what's implemented. Worth clarifying in the desc that this is per-clear_queue!call, not a lifetime/cumulative limit — otherwise the naming suggestion (e.g. something likeevacuate_limit_size_per_clear) is up to you.