hashindex: track unresolved pack location with F_PENDING flag#9828
hashindex: track unresolved pack location with F_PENDING flag#9828mr-raj12 wants to merge 1 commit into
Conversation
Replace the UNKNOWN_BYTES32 pack_id sentinel (in-band signalling) with a ChunkIndex system flag, queried via is_pending(). Follow-up to borgbackup#9821.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9828 +/- ##
==========================================
- Coverage 85.23% 85.18% -0.05%
==========================================
Files 92 92
Lines 15297 15300 +3
Branches 2299 2300 +1
==========================================
- Hits 13038 13034 -4
- Misses 1570 1578 +8
+ Partials 689 688 -1 ☔ View full report in Codecov by Harness. |
| for chunk_id in pending_ids: | ||
| entry = self.chunks.get(chunk_id) | ||
| if entry is not None and entry.pack_id == UNKNOWN_BYTES32: | ||
| if chunk_id in self.chunks and self.chunks.is_pending(chunk_id): |
There was a problem hiding this comment.
aren't they always pending? guess they are until you update_pack_info in 193.
|
Not sure: maybe check whether it would be better to use a user flag for F_PENDING. The slight complication you have in the hashindex code (can't use The "system flags" are rather hashtable-internal things, like tracking what's new and then only writing what's new. |
2 participants
Description
A chunk's pack location (pack_id, obj_offset, obj_size) isn't known yet between put() buffering it in the PackWriter and flush() writing the pack. The old code signalled this by writing the UNKNOWN_BYTES32 sentinel into pack_id and comparing against it later. Since every 256-bit value is a valid pack_id, reserving one as a marker is in-band signalling.
This replaces that with a ChunkIndex system flag, F_PENDING. add() sets it, update_pack_info() clears it once the real location is known, and is_pending() is the query used by repository.list() and get() in place of the old sentinel comparison. UNKNOWN_BYTES32 is still written as filler for the fixed-width record, but nothing reads it for meaning now.
Follow-up to #9821.
Checklist
mastertoxor the relevant test subset)