CLDSRV-936 Lifecycle noncurrent expiration stalls when listing truncates on a bare master#6205
Conversation
Hello nicolas2bert,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Branches have divergedThis pull request's source branch To avoid any integration risks, please re-synchronize them using one of the
Note: If you choose to rebase, you may have to ask me to rebuild |
nicolas2bert
force-pushed
the
bugfix/CLDSRV-936/listLifecycleNonCurrents-baremaster
branch
from
fc1d50e to
1602dc3
Compare
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
nicolas2bert
force-pushed
the
bugfix/CLDSRV-936/listLifecycleNonCurrents-baremaster
branch
2 times, most recently
from
8b35ba5 to
02aaafa
Compare
❌ 3 Tests Failed:
View the full list of 3 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
| // a malformed (non-null) marker that decode rejects by returning an Error | ||
| const result = decodeVersionIdMarker('@@@bad@@@'); | ||
| assert(result instanceof Error); | ||
| assert.strictEqual(result.is.InvalidArgument, true); |
There was a problem hiding this comment.
It could be better to compare value instead of boolean to see in failure message the actual vs expected value instread of false !== true
| assert.strictEqual(result.is.InvalidArgument, true); | |
| assert.strictEqual(result.message, 'InvalidArgument'); |
nicolas2bert
force-pushed
the
bugfix/CLDSRV-936/listLifecycleNonCurrents-baremaster
branch
from
02aaafa to
046ca1a
Compare
…tes on a bare master ++
Apply Prettier formatting to the four files touched by this branch so the prettier:diff CI check passes. Pure formatting, no logic changes.
nicolas2bert
force-pushed
the
bugfix/CLDSRV-936/listLifecycleNonCurrents-baremaster
branch
from
046ca1a to
73e02ed
Compare
nicolas2bert
force-pushed
the
bugfix/CLDSRV-936/listLifecycleNonCurrents-baremaster
branch
from
b5dcbda to
73e02ed
Compare
|
@bert-e approve |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the The following options are set: approve |
|
ping |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: git fetch
git checkout -B w/9.4/bugfix/CLDSRV-936/listLifecycleNonCurrents-baremaster origin/development/9.4
git merge origin/bugfix/CLDSRV-936/listLifecycleNonCurrents-baremaster
# <intense conflict resolution>
git commit
git push -u origin w/9.4/bugfix/CLDSRV-936/listLifecycleNonCurrents-baremasterThe following options are set: approve |
Build failedThe build for commit did not succeed in branch w/9.4/bugfix/CLDSRV-936/listLifecycleNonCurrents-baremaster The following options are set: approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
This pull request does not target the following hotfix branch(es) so they
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
Queue build failedThe corresponding build for the queue failed:
Remove the pull request from the queue
|
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-936. Goodbye nicolas2bert. |
4 participants
The bug in one sentence
The lifecycle noncurrent listing returns the marker version-id-marker=null when a scan page stops on a "bare master" (an object with no internal versionId). CloudServer can't decode 'null', so the worker crashes. The bucket hasn't changed in months, so it crashes every cycle and the noncurrent versions are never expired.
How a bare master (null master) gets created
bucket created, not versioned yet.
PUT foo -> stored as a plain master key "foo", the value has NO versionId field (x-amz-version-id: "null"). <- this is the "bare master"
versioning ENABLED later on the bucket.
foo is NEVER re-PUT -> the bare master stays as-is, forever.
So a bare master = an object written before versioning, never re-PUT afterward. If it had been overwritten, the system would have converted it into a normal null version (id 99999...,isNull2:true), which is harmless. To trigger the bug the bare master never has to be overwritten (which is the case).
How the noncurrent listing works in metadata (bucketd)
The bucket-processor asks CloudServer for a DelimiterNonCurrent listing, which hits bucketd.
bucketd scans the keys in order, newest->oldest per key.
For each key:
the first version seen = the current one -> not returned, but its last-modified is kept in memory.
the following versions = noncurrent -> returned, with a staleDate (= the in memory last-modified).
The staleDate is the last-modified of the version that replaced it, ie the moment that version became noncurrent. It's this staleDate that gets compared to NoncurrentDays. A noncurrent is only returned if staleDate < beforeDate, ie old enough.
Two limits bound each page:
max-keys (default 1000) -> number of noncurrent versions returned.
maxScannedLifecycleListingEntries (default 10000) -> number of entries scanned, so that a bucket full of current objects but with few noncurrents still responds quickly.
When a limit is hit -> IsTruncated=true + a continuation marker (NextKeyMarker + NextVersionIdMarker), and backbeat re-requests the next page with those markers, until IsTruncated=false.
The NextVersionIdMarker is computed as getVersionId() || 'null'. And that's exactly where it goes wrong: on a bare master, getVersionId() is undefined -> the marker becomes the string 'null'.
Example of the issue:
scan# key kind marker becomes
1 bar master (regular) 982... (real id)
2 bar \0 982... current version 982...
3 bar \0 981... non-current -> expirable 981...
... ... ~10k entries, all real ids ...
10000 foo BARE master (no versionId) getVersionId() || 'null' = "null"
>>> STOP (scanned 10000): IsTruncated=true,
NextKeyMarker="foo", NextVersionIdMarker="null" <-- the poison marker
10001 baz ... (next page - never reached)
The noncurrent versions to delete are on the next page, behind that marker.
Condition to trigger the issue: the 10000th scanned entry is a bare master.
If it were a normal version/master, the marker would be a valid id and nothing would break.
NOTE: Only scan-limit truncation can produce 'null', max-keys truncation always stops on a real noncurrent -> real id.
Why it's a bug
The marker round-trip is asymmetric. 'null' is guarded on the way out, not on the way in:
Page 1 version-id-marker= -> returns NextVersionIdMarker:"null" (encode handle 'null' properly-> OK)
Page 2 version-id-marker=null-> decode("null") = Error (decode does NOT handle 'null' properly) -> encode(Error) -> TypeError -> CloudServer worker crash.
bucketd answers page 2 without any problem (HTTP 200). The crash is 100% on the CloudServer side, while re-encoding the version-id-marker=null from the request itself.
Why it stays stuck
The processor gives up after its retries and moves on to something else. But the versions to expire are on page 2, which crashes every time. The bucket doesn't change -> every cycle page 1 re-emits 'null', page 2 re-crashes.
Net progress = zero, indefinitely -> "noncurrent versions never removed".
the fix : treat version-id-marker=null as "no marker" instead of blindly decoding it.