Fix crash when active field-expiry leaves a single-entry HT vset bucket#3950
Conversation
When a hash has many fields whose expirations fall in the same volatile-set time-bucket (>127 entries), that bucket is encoded as a hashtable (HT). The active field-expiry path drains expired entries via vsetBucketRemoveExpired_HASHTABLE(), but it is bounded by the per-key expire quota (max_count, from dbReclaimExpiredFields). When the quota stops the drain one entry short, the function only collapsed the bucket to NONE at size 0 and left it as an HT bucket holding a single entry. That violates the invariant enforced by removeFromBucket_HASHTABLE(), which asserts hashtableSize(ht) > 0 after a delete and downgrades an HT bucket to SINGLE at size 1. A later normal removal of that lone field -- HDEL, or a cross-bucket HSETEX update routed through removeEntryFromRaxBucket() -- deletes the sole entry, drops the size 1 -> 0, and trips the assertion. Fix: Apply the same downgrade in the expiry path: when draining leaves exactly one entry, downgrade the HT bucket to SINGLE so the invariant holds. Behavior is unchanged when the bucket drains fully (-> NONE) or retains two or more entries (-> HT, as the normal removal path already tolerates). Add a regression test that fills one time-bucket past the HT threshold, expires all but one entry, then removes the survivor through the normal path; it crashed before this fix and passes after. Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR fixes an invariant violation in vset bucket transitions by collapsing or downgrading hashtable-encoded buckets after expired entry removal. The expired-entry drain path now checks the remaining hashtable size and either releases the hashtable entirely (setting bucket to NONE) or downgrades it to SINGLE type (extracting the last entry). A regression test validates the specific scenario where expiration leaves one entry, then removes it via the normal path. ChangesHashtable bucket collapse on expired entry removal
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/unit/test_vset.cppsrc/unit/test_vset.cpp:7:10: fatal error: 'generated_wrappers.hpp' file not found ... [truncated 1085 characters] ... lang/install/lib/clang/18/include" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #3950 +/- ##
============================================
- Coverage 76.66% 76.57% -0.09%
============================================
Files 162 162
Lines 80733 80757 +24
============================================
- Hits 61895 61842 -53
- Misses 18838 18915 +77
🚀 New features to boost your workflow:
|
|
Tnank you @enjoy-binbin ! |
#3957) backport of (#3950) When a hash has many fields whose expirations fall in the same volatile-set time-bucket (>127 entries), that bucket is encoded as a hashtable (HT). The active field-expiry path drains expired entries via vsetBucketRemoveExpired_HASHTABLE(), but it is bounded by the per-key expire quota (max_count, from dbReclaimExpiredFields). When the quota stops the drain one entry short, the function only collapsed the bucket to NONE at size 0 and left it as an HT bucket holding a single entry. That violates the invariant enforced by removeFromBucket_HASHTABLE(), which asserts hashtableSize(ht) > 0 after a delete and downgrades an HT bucket to SINGLE at size 1. A later normal removal of that lone field -- HDEL, or a cross-bucket HSETEX update routed through removeEntryFromRaxBucket() -- deletes the sole entry, drops the size 1 -> 0, and trips the assertion. Fix: Apply the same downgrade in the expiry path: when draining leaves exactly one entry, downgrade the HT bucket to SINGLE so the invariant holds. Behavior is unchanged when the bucket drains fully (-> NONE) or retains two or more entries (-> HT, as the normal removal path already tolerates). Add a regression test that fills one time-bucket past the HT threshold, expires all but one entry, then removes the survivor through the normal path; it crashed before this fix and passes after. Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
#3958) backport of (#3950) When a hash has many fields whose expirations fall in the same volatile-set time-bucket (>127 entries), that bucket is encoded as a hashtable (HT). The active field-expiry path drains expired entries via vsetBucketRemoveExpired_HASHTABLE(), but it is bounded by the per-key expire quota (max_count, from dbReclaimExpiredFields). When the quota stops the drain one entry short, the function only collapsed the bucket to NONE at size 0 and left it as an HT bucket holding a single entry. That violates the invariant enforced by removeFromBucket_HASHTABLE(), which asserts hashtableSize(ht) > 0 after a delete and downgrades an HT bucket to SINGLE at size 1. A later normal removal of that lone field -- HDEL, or a cross-bucket HSETEX update routed through removeEntryFromRaxBucket() -- deletes the sole entry, drops the size 1 -> 0, and trips the assertion. Fix: Apply the same downgrade in the expiry path: when draining leaves exactly one entry, downgrade the HT bucket to SINGLE so the invariant holds. Behavior is unchanged when the bucket drains fully (-> NONE) or retains two or more entries (-> HT, as the normal removal path already tolerates). Add a regression test that fills one time-bucket past the HT threshold, expires all but one entry, then removes the survivor through the normal path; it crashed before this fix and passes after. Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
…et (#3950) When a hash has many fields whose expirations fall in the same volatile-set time-bucket (>127 entries), that bucket is encoded as a hashtable (HT). The active field-expiry path drains expired entries via vsetBucketRemoveExpired_HASHTABLE(), but it is bounded by the per-key expire quota (max_count, from dbReclaimExpiredFields). When the quota stops the drain one entry short, the function only collapsed the bucket to NONE at size 0 and left it as an HT bucket holding a single entry. That violates the invariant enforced by removeFromBucket_HASHTABLE(), which asserts hashtableSize(ht) > 0 after a delete and downgrades an HT bucket to SINGLE at size 1. A later normal removal of that lone field -- HDEL, or a cross-bucket HSETEX update routed through removeEntryFromRaxBucket() -- deletes the sole entry, drops the size 1 -> 0, and trips the assertion. Fix: Apply the same downgrade in the expiry path: when draining leaves exactly one entry, downgrade the HT bucket to SINGLE so the invariant holds. Behavior is unchanged when the bucket drains fully (-> NONE) or retains two or more entries (-> HT, as the normal removal path already tolerates). Add a regression test that fills one time-bucket past the HT threshold, expires all but one entry, then removes the survivor through the normal path; it crashed before this fix and passes after. Signed-off-by: Ran Shidlansik <ranshid@amazon.com>
When a hash has many fields whose expirations fall in the same volatile-set time-bucket (>127 entries), that bucket is encoded as a hashtable (HT). The active field-expiry path drains expired entries via vsetBucketRemoveExpired_HASHTABLE(), but it is bounded by the per-key expire quota (max_count, from dbReclaimExpiredFields). When the quota stops the drain one entry short, the function only collapsed the bucket to NONE at size 0 and left it as an HT bucket holding a single entry.
That violates the invariant enforced by removeFromBucket_HASHTABLE(), which asserts hashtableSize(ht) > 0 after a delete and downgrades an HT bucket to SINGLE at size 1. A later normal removal of that lone field -- HDEL, or a cross-bucket HSETEX update routed through removeEntryFromRaxBucket() -- deletes the sole entry, drops the size 1 -> 0, and trips the assertion.
Fix: Apply the same downgrade in the expiry path: when draining leaves exactly one entry, downgrade the HT bucket to SINGLE so the invariant holds. Behavior is unchanged when the bucket drains fully (-> NONE) or retains two or more entries (-> HT, as the normal removal path already tolerates).
Add a regression test that fills one time-bucket past the HT threshold, expires all but one entry, then removes the survivor through the normal path; it crashed before this fix and passes after.