echonet: store cende blobs as raw bytes with tighter in-memory retention#14712
echonet: store cende blobs as raw bytes with tighter in-memory retention#14712ron-starkware wants to merge 1 commit into
Conversation
|
This change is |
ron-starkware
force-pushed
the
ron/echonet-os/deploy-recreate
branch
from
515759e to
93c9de8
Compare
ron-starkware
force-pushed
the
ron/echonet-os/blob-bytes-store
branch
3 times, most recently
from
691f48d to
ea3e483
Compare
PR SummaryMedium Risk Overview Retention and archiving now evicts in-memory Serving: Reviewed by Cursor Bugbot for commit 7dc81ee. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ea3e483. Configure here.
| if evicted_blob_bodies: | ||
| _BlockStore.write_blob_bodies_to_disk( | ||
| evicted_blob_bodies, base_dir=self._blocks._ensure_archive_dir() | ||
| ) |
There was a problem hiding this comment.
Blob dropped before durable archive
Medium Severity
Blob bodies are removed from the in-memory block entry before write_blob_bodies_to_disk runs outside the lock. If that write fails or a reader runs in that gap, the blob is gone from memory while block metadata remains. Later full-block snapshots skip writing a blob file when blob_body was already popped, so there is no second chance to persist it.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit ea3e483. Configure here.
| """Return the raw blob bytes for `block_number`, from memory or the on-disk archive.""" | ||
| in_mem = self.get_block_field(block_number, "blob_body") | ||
| if in_mem: | ||
| return in_mem |
There was a problem hiding this comment.
Empty blob treated as missing
Low Severity
get_blob_body_with_disk_fallback and the block-dump handler treat a stored blob_body of empty bytes as falsy, so they skip returning it and respond with not found instead of serving the empty payload.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ea3e483. Configure here.
ron-starkware
left a comment
ron-starkware
left a comment
There was a problem hiding this comment.
@ron-starkware resolved 2 discussions.
Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on matanl-starkware).
A 6 MB blob parsed into a Python dict costs 30-50 MB of heap; at the current retention window the parsed-dict store OOMs the pod. Keep the raw JSON bytes instead (consumers parse just-in-time), evict blob bodies from memory on a much tighter window than block documents, and archive evicted bodies to the PVC, serving them back through a new get_blob_body_with_disk_fallback (also used by the block-dump endpoint's blob kind). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ron-starkware
force-pushed
the
ron/echonet-os/blob-bytes-store
branch
from
ea3e483 to
7dc81ee
Compare
2 participants
A 6 MB blob parsed into a Python dict costs 30-50 MB of heap; at the
current retention window the parsed-dict store OOMs the pod. Keep the
raw JSON bytes instead (consumers parse just-in-time), evict blob
bodies from memory on a much tighter window than block documents, and
archive evicted bodies to the PVC, serving them back through a new
get_blob_body_with_disk_fallback (also used by the block-dump
endpoint's blob kind).
Co-Authored-By: Claude Fable 5 noreply@anthropic.com