DAOS-19451 cont: enable checksum by default on non-v0 pools - #18885
ryon-jensen wants to merge 1 commit into
Conversation
|
Test stage NLT completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/1/testReport/ |
|
Ticket title is 'container checksum is off by default' |
|
Test stage Functional Cluster Box Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/2/execution/node/1522/log |
|
Test stage Functional Cluster Box Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/3/execution/node/1525/log |
|
Test stage Functional Cluster Box Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/4/execution/node/1524/log |
d2e8bf5 to
e842977
Compare
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/5/testReport/ |
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/6/testReport/ |
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/6/execution/node/1632/log |
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/6/execution/node/1622/log |
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/12/execution/node/1623/log |
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/12/testReport/ |
7c9f696 to
0b011e1
Compare
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/13/testReport/ |
0b011e1 to
3d6584c
Compare
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/14/testReport/ |
fa0550e to
495761b
Compare
|
Test stage Functional Cluster Box Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/21/execution/node/1672/log |
495761b to
6769097
Compare
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/21/execution/node/1725/log |
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/22/testReport/ |
6769097 to
bba33d3
Compare
|
Test stage NLT completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-18885/24/display/redirect |
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/25/testReport/ |
bba33d3 to
db6608d
Compare
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/26/testReport/ |
db6608d to
16cc4a5
Compare
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/27/testReport/ |
6c72ccb to
8fc84b3
Compare
|
Test stage Functional Hardware Large MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/28/execution/node/1759/log |
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/28/execution/node/1637/log |
| @@ -367,7 +367,7 @@ def __init__(self, pool, daos_command, label_generator=None, namespace=CONT_NAME | |||
| self.dir_oclass = BasicParameter(None) | |||
| self.file_oclass = BasicParameter(None) | |||
| self.chunk_size = BasicParameter(None) | |||
| self.properties = BasicParameter(None, "cksum:off,srv_cksum:off") | |||
| self.properties = BasicParameter(None) | |||
There was a problem hiding this comment.
This particular change is being addressed as part of https://daosio.atlassian.net/browse/DAOS-19452, in particular in https://daosio.atlassian.net/browse/DAOS-19488 with #18851.
|
Test stage Functional Cluster Box Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18885/31/testReport/ |
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18885/31/execution/node/1719/log |
8fc84b3 to
7266c80
Compare
DAOS-17946 changed the default container properties to crc32 checksum and server-side checksum verification, but only updated the v0 default property table in src/container/srv_layout.c. cont_prop_entries_default, the table used for every pool with a global version >= 1, was left untouched, so containers created on any modern pool still defaulted to checksum off. Apply the same checksum defaults to cont_prop_entries_default: crc32 checksum and server-side verification on. The v0 table also defaults the redundancy factor to RF3, but that is deliberately not replicated here. A container inherits the redundancy factor from the pool whenever it is not specified, so the entry would change nothing in practice while leaving a surprising default sitting in the table. The regression went unnoticed because the test harnesses hid it: daos_test forced checksum off on every container it created and the ftest container utility hardcoded "cksum:off,srv_cksum:off", both added by DAOS-17946. Drop those overrides and update the co_properties assertions so the defaults are actually exercised. Enabling the default reactivated a large amount of checksum code that no test had been exercising, surfacing a series of pre-existing defects that block it: - cont: unlink the container from the pool list when dc_cont_props_init() fails, so the error path does not leave a dangling entry. - object: free partially decoded checksums on both iod_csums decode error paths. - object: fix an engine crash in CPD update. LOCAL_STACK_NUM is 2, so an update writing more than two iods heap-allocates pcsums with NULL elements that obj_get_iods_offs() dereferenced unconditionally. - object: fix client checksum verify for echo RPCs, which short-circuit before obj_fetch_create_maps() and tripped the map-count assertion, plus a zero-length iov guard. - common: skip zero-length iovs in daos_sgl_get_bytes(). A size-query fetch sets every iov_len to zero, tripping idx->iov_offset < len as 0 < 0. - object: do not verify fetches that transferred no data. - object: inject DAOS_CSUM_CORRUPT_DISK after obj_verify_bio_csum(). It fakes silent on-disk corruption, but with server-side verify now on the write was rejected before reaching disk, breaking the scrubber tests. - cont, object: report the return code with DF_RC when container property initialisation or key checksum verification fails. NLT suppresses fault-injection errors that carry DER_NOMEM, so these paths were reported as new warnings once enabling checksums made them reachable. The subtlest defect is the EC single-value layout used to verify a fetch. On a fetch vos_fetch_begin() overwrites iod_size with the stored record size before obj_singv_ec_rw_filter() runs, so the server lays the reply out and checksums it according to the size it replies with, and each shard replies its own size. The client request was built from the size the caller asked with, which for DAOS_REC_ANY is only a guess taken from the sgl buffer size and may be an over-estimate otherwise. Two problems followed. dc_rw_cb_csum_verify() is called per shard but receives orw_iod_array.oia_iods, which for an EC request aliases the shared reasb_req->orr_iods, and it completed the equally shared orr_singv_los[] in place, so one shard callback could overwrite the layout another shard was about to verify against. Independently, when the requested and replied sizes fall on opposite sides of the even distribution threshold, or merely imply different cell sizes, the reply does not describe the record the request was segmented for, and verifying it reported a false corruption. That fed obj_ec_fail_info_insert() and surfaced as -DER_DATA_LOSS once the parity limit was exceeded. Derive the layout from the size this shard replied, which is already used for the record length the checksum is calculated over, and return it through a caller owned struct instead of completing the shared one, so concurrent shard callbacks cannot interfere with each other. When the replied size implies a different layout than the request was built for, return -DER_FETCH_AGAIN. obj_reasb_io_fini() restores the user iods for that error and dc_shard_update_size() has already stored the replied size in them, so the retry is assembled for the layout the server actually used and is verified normally. Verification is deferred, never skipped or downgraded, so a real checksum error is still reported. That layout fix alone was not sufficient. The singv size conflict that forces the retry is derived from reasb_req->orr_fetch_stat, which holds the sizes replied by all shards, but it was only evaluated on whichever branch handled the reply in hand, and the parity/small-index branch additionally required every other shard to have replied success. A -DER_REC2BIG from another shard is the long singv overwritten by short singv case, which is deliberately ignored when the short value lives on a single target, so the conflict was never evaluated and the callback went on to verify against a stale layout. Evaluate the conflict from the shared fetch statistics on every callback, and accept -DER_REC2BIG as a state in which it still applies. EC17 reproduced once in four runs before this change and passed six consecutive runs after it, with no checksum mismatch logged in any of them. Features: checksum Test-tag: daos_test pr test_daos_container checksum Signed-off-by: Ryon Jensen <ryon.jensen@hpe.com>
7266c80 to
1a600f8
Compare
DAOS-17946 changed the default container properties to crc32 checksum and server-side checksum verification, but only updated the v0 default property table in src/container/srv_layout.c. cont_prop_entries_default, which is the table used for every pool with a global version >= 1, was left untouched, so containers created on any modern pool still defaulted to checksum off.
Apply the same defaults to cont_prop_entries_default: crc32 checksum, server-side verification on and RF3. Note that the container redundancy factor is always inherited from the pool when it is not explicitly specified, so the RF3 entry is only there to keep the two tables consistent.
The regression went unnoticed because the test harnesses hid it: daos_test forced checksum off on every container it created and the ftest container utility hardcoded "cksum:off,srv_cksum:off", both added by DAOS-17946. Drop those overrides and update the co_properties assertions so the defaults are actually exercised.
Latent checksum bugs uncovered by the new default
Turning checksums on reactivated a large amount of code that no test had been
exercising, and CI surfaced a series of pre-existing defects that are unrelated
to the default itself but block it:
contdc_cont_props_init()became fallible, butcont_open_complete()had already linked the container into the pool list and never unlinked it on failure, so the error path left a dangling entry.objectiod_csumsdecode error paths leaked the partially decoded array.objectLOCAL_STACK_NUMis 2, so a CPD update writing more than two iods heap-allocates thepcsumsarray with NULL elements;obj_get_iods_offs()dereferenced one unconditionally and crashed the engine.objectobj_fetch_create_maps(), so the client map-count assertion fired; plus a zero-length iov guard.commoniov_lento zero, trippingidx->iov_offset < lenwith0 < 0indaos_sgl_get_bytes().objectobjectobjectDAOS_CSUM_CORRUPT_DISKfakes silent on-disk corruption, but was injected beforeobj_verify_bio_csum(). With server-side verify now on, the write was rejected and nothing reached disk, breaking the scrubber tests. Moved the injection after verification.The subtlest one is the EC single-value layout used to verify a fetch.
On a fetch
vos_fetch_begin()overwritesiod_sizewith the stored record sizebefore
obj_singv_ec_rw_filter()runs, so the server lays the reply out andchecksums it according to the size it replies with, and each shard replies its
own size. The client request was built from the size the caller asked with,
which for
DAOS_REC_ANYis only a guess taken from the sgl buffer size, and maybe an over-estimate otherwise.
Two problems followed:
dc_rw_cb_csum_verify()runs pershard but receives
orw_iod_array.oia_iods, which for an EC request aliasesthe shared
reasb_req->orr_iods, and it completed the equally sharedorr_singv_los[]in place.dc_shard_update_size()writes that sameshared
iod_sizefrom each reply, so one shard callback could overwrite thesize and layout another shard was about to verify against.
of the even-distribution threshold, or merely imply different cell sizes, the
reply does not describe the record the request was segmented for.
Either way verification reported a false corruption, which fed
obj_ec_fail_info_insert()and surfaced as-DER_DATA_LOSSonce the paritylimit was exceeded.
EC17reproduces both, logging the straddling pair:The fix derives the layout from the size this shard replied — the value
already used as the record length the checksum is calculated over — and returns
it through a caller-owned struct rather than completing the shared one. When the
replied size implies a different layout than the request was built for, it
returns
-DER_FETCH_AGAIN:obj_reasb_io_fini()restores the user iods forthat error and
dc_shard_update_size()has already stored the replied size inthem, so the retry is assembled for the layout the server actually used and is
verified normally. Verification is deferred, never skipped or downgraded, so a
real checksum error is still reported.
Verified on a six-engine single-host configuration, which is enough ranks for
the EC suite to run: four consecutive full
daos_test -Iruns pass 30/30 (thefailure reproduced roughly every second run beforehand),
daos_test -Xpasses29/29, and
daos_test -Tpasses the distributed TX suite.Steps for the author:
After all prior steps are complete: