Repository navigation
Conversation
|
Ticket title is 'Checksum management with ddb' |
Adds dv_check_csum() to the ddb VOS API (ddb_vos.c/.h), mirroring the structure of the existing dv_dump_csum() (open container, build the iod from path, dispatch by value type, close container) as an independent implementation that additionally fetches the live data, recomputes its checksum(s), and compares against what was fetched. Results are reported through a callback with a got_csums array: got_csums[i] holds the recomputed checksum when entry i's stored checksum did not match, and is NULL when it matched or there was nothing to check. The function returns -DER_CSUM if any checksum entry fails to match. Test coverage: unit tests for dv_check_csum() (single value and array, matching/mismatching/no-checksum cases, and per-entry mismatch isolation when one of several overlapping array segments is corrupted), using extended VOS test fixtures (ddb_test_driver.c/.h) for corrupted single-value and array checksums. This is patch 1/3 of the DAOS-17321 csum_check split (supersedes PR #18940): see patch-007 for the ddb command wiring and patch-008 for the Go CLI/documentation. Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
243f712 to
d64d9da
Compare
|
Test stage Functional Cluster Box 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-18941/1/execution/node/1599/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-18941/1/execution/node/1588/log |
|
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-18941/2/execution/node/1724/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-18941/2/execution/node/1821/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-18941/2/execution/node/1874/log |
…/daos-17321/patch-006 Features: recovery
…/daos-17321/patch-006 Features: recovery
|
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-18941/4/execution/node/1852/log |
|
The only failure recovery/check_start_corner_case.py:DMGCheckStartCornerCaseTest.test_two_pools_corrupted of the last build #4 is a known issue already tracked by the ticket DAOS-18773 |
…/daos-17321/patch-006
The checksum check and dump paths of the ddb VOS API asserted that a VOS_OF_FETCH_CSUM fetch returns one checksum info per stored extent of an array akey and exactly one for a single value. This holds on a healthy tree, but corrupted checksum metadata can break it, and a checksum info carries no back-reference to its extent, so the two lists cannot be re-paired. A recovery tool must not abort on the data it inspects. check_csum_recx(), dump_csum_recx(), check_csum_sv() and dump_csum_sv() now check the count explicitly and, on a mismatch, log the expected and actual counts and return -DER_CSUM without invoking the callback, as done for a data mismatch. A DDB_CSUM_NR_INJECT fault-injection point forges the count so that ddb_tests covers the four paths on a real pool; these tests skip on builds without FAULT_INJECTION. Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…/daos-17321/patch-006 Features: recovery
verify_segment_csum() reports a checksum mismatch with -DER_CSUM and a snapshot of the recomputed checksum in *got_csum. A non-NULL snapshot is the mismatch signal handed to the dv_check_csum() callback, and check_csum_sv() derives its result from it. When the snapshot could not be allocated, the allocation error was returned but the detected mismatch was not logged, and so was lost. Log the mismatch together with the allocation error and keep returning that error: returning -DER_CSUM with a NULL snapshot would make the callback report the entry as matching and check_csum_sv() return success. Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
|
Test stage Functional on EL 9 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-18941/6/execution/node/1280/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-18941/7/execution/node/1696/log |
|
|
README-fi.md previously only covered pairing an FI point with a Python functional test. Extend it with a full C-unit-test path (FAULT_INJECTION_REQUIRED(), arming/disarming fixtures, parameter channel differences between C/RPC and YAML), using the ddb DDB_CSUM_NR_INJECT checksum-consistency hook (PR #18941) as the worked example alongside the existing PR #18489 ftest example. Also corrects a stale claim (d_fault_inject is unsigned int, not bool) and documents the two DAOS FI idioms (DAOS_FAIL_CHECK() vs. direct d_fault_attr_lookup()+D_SHOULD_FAIL()), where FI tests run in CI, and the low-16-bit ID overlap between the unit-test and system-test groups. Verified against ~/work/daos source: bit-mask/type claims, DDB_CSUM_NR_INJECT call sites, FAULT_INJECTION_REQUIRED(), TEST_CSUM_FI fixtures, utest.yaml/Jenkinsfile/GHA references all match; fixed one stale illustrative offset (0x300 -> 0x400) to match the actual constant in src/include/daos/common.h. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…/daos-17321/patch-006 Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…not report fetch_check_value() treated a zero-size fetch result as a hole or a punch and returned success with an empty sgl. verify_segment_csum() then handed an iod with iod_size == 0 to daos_csummer_calc_iods(), which allocates no checksum for such an iod (csum_iod_is_supported() is false), so ic_data stayed NULL and daos_csummer_compare_csum_info() dereferenced it. The hole/punch assumption was wrong: VOS never records holes or punches in the checksum-fetch results. akey_fetch_recx() skips hole extents without calling save_recx() or save_csum(), and svt_rec_load() loads no checksum for a punch record. Every segment reaching fetch_check_value() is thus backed by a stored checksum, and a zero-size result means a stored checksum with no data behind it, i.e. inconsistent checksum metadata (reachable today when akey_fetch_single() saves the checksum of a single value older than the akey re-creation epoch and leaves the SV epoch at 0). fetch_check_value() now logs the inconsistency and returns -DER_NONEXIST, which check_csum_sv() and check_csum_recx() map to -DER_CSUM without invoking the callback, in line with the existing checksum-info count consistency checks and the dv_check_csum() contract. verify_segment_csum() likewise returned the allocation error when a mismatch was detected but the recomputed checksum could not be snapshotted for the callback, so the check failed with e.g. -DER_NOMEM and gave no hint of the corruption that was found. The mismatch is now logged and returned as -DER_CSUM with a NULL got_csum, which check_csum_sv() and check_csum_recx() report to their caller without invoking the callback: the dv_check_csum_cb contract (got_csums[i] != NULL is the mismatch flag of entry i) is preserved and the root cause is not masked by the secondary failure. Add the DDB_CSUM_NO_DATA_INJECT and DDB_CSUM_SNAPSHOT_NOMEM_INJECT fault locations to forge such a segment and such an allocation failure in the ddb_tests fault-injection tests for both single values and arrays. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
|
Test stage Unit Test completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-18941/8/display/redirect |
|
Test stage Unit Test with memcheck completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-18941/8/display/redirect |
|
Test stage NLT completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-18941/8/display/redirect |
|
Test stage Unit Test bdev completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-18941/8/display/redirect |
@NiuYawei , @janekmi and @Nasf-Fan, the PR is now ready for review. Sadly, the size of the PR noticeably increases for being able to properly check that inconsistent VOS evtree are properly managed: the production change is ~140 lines, the rest is tests/fixtures. |
README-fi.md previously only covered pairing an FI point with a Python functional test. Extend it with a full C-unit-test path (FAULT_INJECTION_REQUIRED(), arming/disarming fixtures, parameter channel differences between C/RPC and YAML), using the ddb DDB_CSUM_NR_INJECT checksum-consistency hook (PR #18941) as the worked example alongside the existing PR #18489 ftest example. Also corrects a stale claim (d_fault_inject is unsigned int, not bool) and documents the two DAOS FI idioms (DAOS_FAIL_CHECK() vs. direct d_fault_attr_lookup()+D_SHOULD_FAIL()), where FI tests run in CI, and the low-16-bit ID overlap between the unit-test and system-test groups. Verified against ~/work/daos source: bit-mask/type claims, DDB_CSUM_NR_INJECT call sites, FAULT_INJECTION_REQUIRED(), TEST_CSUM_FI fixtures, utest.yaml/Jenkinsfile/GHA references all match; fixed one stale illustrative offset (0x300 -> 0x400) to match the actual constant in src/include/daos/common.h. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
|
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-18941/9/execution/node/1745/log |
…/daos-17321/patch-006
fetch_check_value() returned -DER_NONEXIST when a checksummed value has no data behind it (inconsistent metadata), requiring check_csum_sv() and check_csum_recx() to each carry a duplicated "if (rc == -DER_NONEXIST) rc = -DER_CSUM" translation block. csum_recx_check_chunks(), the sibling helper that detects a checksum not covering exactly the chunks of its extent, already returns -DER_CSUM directly for its own inconsistency, and its callers simply propagate it with a plain "if (!SUCCESS(rc))". fetch_check_value() was the outlier. fetch_check_value() now returns -DER_CSUM directly for this case, matching csum_recx_check_chunks() and the dv_check_csum() contract already documented in ddb_vos.h. The now-redundant translation blocks in check_csum_sv() and check_csum_recx() are removed: their existing "rc == -DER_CSUM && got_csum(s) == NULL" handling already covers it. Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…/daos-17321/patch-006 Features: recovery
| D_GOTO(out_fetch_end, rc = -DER_CSUM); | ||
| } | ||
| ci = dcs_csum_info_get(cil, 0); | ||
| D_ASSERT(ci_is_valid(ci)); |
There was a problem hiding this comment.
Be as backend debug tools, it is better not assert value from disk.
There was a problem hiding this comment.
- Remove invalid assert
There was a problem hiding this comment.
The invalid assertion is still here after refresh. Please consider to change. The others looks fine for me.
There was a problem hiding this comment.
I have freeze this PR until the PR #19177 has been landed.
During the investigation of this concerns, I find several other similar issues which makes the code to grow significantly and thus I decided to split it.
More details in the comment #18941 (comment)
| daos_iod_t seg_iod; | ||
|
|
||
| ci = dcs_csum_info_get(cil, i); | ||
| D_ASSERT(ci_is_valid(ci)); |
There was a problem hiding this comment.
Be as backend debug tools, it is better not assert value from disk.
There was a problem hiding this comment.
- Remove invalid assert
|
This PR has grown too big (about +2100/−150 lines, mostly tests), so I am splitting it as I did for #19177 : the
|
README-fi.md previously only covered pairing an FI point with a Python functional test. Extend it with a full C-unit-test path (FAULT_INJECTION_REQUIRED(), arming/disarming fixtures, parameter channel differences between C/RPC and YAML), using the ddb DDB_CSUM_NR_INJECT checksum-consistency hook (PR #18941) as the worked example alongside the existing PR #18489 ftest example. Also corrects a stale claim (d_fault_inject is unsigned int, not bool) and documents the two DAOS FI idioms (DAOS_FAIL_CHECK() vs. direct d_fault_attr_lookup()+D_SHOULD_FAIL()), where FI tests run in CI, and the low-16-bit ID overlap between the unit-test and system-test groups. Verified against ~/work/daos source: bit-mask/type claims, DDB_CSUM_NR_INJECT call sites, FAULT_INJECTION_REQUIRED(), TEST_CSUM_FI fixtures, utest.yaml/Jenkinsfile/GHA references all match; fixed one stale illustrative offset (0x300 -> 0x400) to match the actual constant in src/include/daos/common.h. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…asserting ddb is a recovery tool and must not abort on checksum metadata read from a possibly damaged pool. The check paths of the ddb VOS API asserted that the checksum infos returned by VOS are valid (ci_is_valid()) and that the recx list exists, and the dump paths handed them to the callbacks without any check, so a corrupted pool would crash ddb instead of reporting the error. Add csum_info_check(), which validates the fields of a checksum info and its checksum type (also read from the pool and otherwise dereferenced through daos_mhash_type2algo() by the callbacks), and use it in dump_csum_sv(), dump_csum_recx(), check_csum_sv() and check_csum_recx(). Handle a missing recx list (no extent stored in the requested range/epoch) in both recx paths and reshape dump_csum_recx() like check_csum_recx(). Every inconsistency is logged and returned as -DER_CSUM before any callback is invoked; the resulting callback contract is documented in ddb_vos.h. Add the DDB_CSUM_TYPE_INJECT fault injection point and extend the four *_inconsistent_tests of ddb_tests with the invalid checksum info, missing recx list and unknown checksum type cases, plus the non-FI "no stored extent" cases of the dump and check recx tests. Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
ddb is a recovery tool: the checksum metadata it reads from a pool may be damaged and must be neither trusted nor asserted on. dv_dump_csum() handed the checksum infos returned by VOS straight to its callback, which asserted that the checksum-info count matches the extent count and that every checksum info is valid, and dereferenced the checksum type through daos_mhash_type2algo() without checking it. Validate the fetched metadata in dv_dump_csum() before any callback is invoked: the checksum-info count must be zero or match the stored extents (recx list present), every checksum info must pass ci_is_valid(), its type must be known to the checksum library and its chunk count must cover exactly its extent. Any inconsistency is logged and returned as -DER_CSUM; the callback contract is documented in ddb_vos.h. Add the DDB_CSUM_NR_INJECT, DDB_CSUM_CHUNK_NR_INJECT and DDB_CSUM_TYPE_INJECT fault injection points, ddb_tests fixtures with partially overwritten extents, and the corresponding dump_csum tests (dv_dump_csum() and the print/write csum_dump callbacks). Split off from #18941, which keeps the dv_check_csum() API. Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
README-fi.md previously only covered pairing an FI point with a Python functional test. Extend it with a full C-unit-test path (FAULT_INJECTION_REQUIRED(), arming/disarming fixtures, parameter channel differences between C/RPC and YAML), using the ddb DDB_CSUM_NR_INJECT checksum-consistency hook (PR #18941) as the worked example alongside the existing PR #18489 ftest example. Also corrects a stale claim (d_fault_inject is unsigned int, not bool) and documents the two DAOS FI idioms (DAOS_FAIL_CHECK() vs. direct d_fault_attr_lookup()+D_SHOULD_FAIL()), where FI tests run in CI, and the low-16-bit ID overlap between the unit-test and system-test groups. Verified against ~/work/daos source: bit-mask/type claims, DDB_CSUM_NR_INJECT call sites, FAULT_INJECTION_REQUIRED(), TEST_CSUM_FI fixtures, utest.yaml/Jenkinsfile/GHA references all match; fixed one stale illustrative offset (0x300 -> 0x400) to match the actual constant in src/include/daos/common.h. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
README-fi.md previously only covered pairing an FI point with a Python functional test. Extend it with a full C-unit-test path (FAULT_INJECTION_REQUIRED(), arming/disarming fixtures, parameter channel differences between C/RPC and YAML), using the ddb DDB_CSUM_NR_INJECT checksum-consistency hook (PR #18941) as the worked example alongside the existing PR #18489 ftest example. Also corrects a stale claim (d_fault_inject is unsigned int, not bool) and documents the two DAOS FI idioms (DAOS_FAIL_CHECK() vs. direct d_fault_attr_lookup()+D_SHOULD_FAIL()), where FI tests run in CI, and the low-16-bit ID overlap between the unit-test and system-test groups. Verified against ~/work/daos source: bit-mask/type claims, DDB_CSUM_NR_INJECT call sites, FAULT_INJECTION_REQUIRED(), TEST_CSUM_FI fixtures, utest.yaml/Jenkinsfile/GHA references all match; fixed one stale illustrative offset (0x300 -> 0x400) to match the actual constant in src/include/daos/common.h. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
README-fi.md previously only covered pairing an FI point with a Python functional test. Extend it with a full C-unit-test path (FAULT_INJECTION_REQUIRED(), arming/disarming fixtures, parameter channel differences between C/RPC and YAML), using the ddb DDB_CSUM_NR_INJECT checksum-consistency hook (PR #18941) as the worked example alongside the existing PR #18489 ftest example. Also corrects a stale claim (d_fault_inject is unsigned int, not bool) and documents the two DAOS FI idioms (DAOS_FAIL_CHECK() vs. direct d_fault_attr_lookup()+D_SHOULD_FAIL()), where FI tests run in CI, and the low-16-bit ID overlap between the unit-test and system-test groups. Verified against ~/work/daos source: bit-mask/type claims, DDB_CSUM_NR_INJECT call sites, FAULT_INJECTION_REQUIRED(), TEST_CSUM_FI fixtures, utest.yaml/Jenkinsfile/GHA references all match; fixed one stale illustrative offset (0x300 -> 0x400) to match the actual constant in src/include/daos/common.h. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…atch-009' into ckochhof/dev/master/daos-17321/patch-006 Features: recovery Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
|
@Nasf-Fan , @NiuYawei and @janekmi This PR is now based on the branch of #19177 (the Commit 13114e4 addresses the two open Once #19177 has landed I will merge master here; the diff should not change. |
|
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/job/PR-18941/13/display/redirect |
|
Test stage Functional Hardware Medium 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-18941/13/execution/node/2061/log |
README-fi.md previously only covered pairing an FI point with a Python functional test. Extend it with a full C-unit-test path (FAULT_INJECTION_REQUIRED(), arming/disarming fixtures, parameter channel differences between C/RPC and YAML), using the ddb DDB_CSUM_NR_INJECT checksum-consistency hook (PR #18941) as the worked example alongside the existing PR #18489 ftest example. Also corrects a stale claim (d_fault_inject is unsigned int, not bool) and documents the two DAOS FI idioms (DAOS_FAIL_CHECK() vs. direct d_fault_attr_lookup()+D_SHOULD_FAIL()), where FI tests run in CI, and the low-16-bit ID overlap between the unit-test and system-test groups. Verified against ~/work/daos source: bit-mask/type claims, DDB_CSUM_NR_INJECT call sites, FAULT_INJECTION_REQUIRED(), TEST_CSUM_FI fixtures, utest.yaml/Jenkinsfile/GHA references all match; fixed one stale illustrative offset (0x300 -> 0x400) to match the actual constant in src/include/daos/common.h. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
This patch is part of a 3-way split of #18940 ("DAOS-17321 ddb: add csum_check command to verify data checksums"), which was too large to review as a single PR. The split mirrors the layering already used for the sibling
csum_dumpfeature earlier in this same ticket (#18444 -> #18543 -> #18685):dv_check_csum()to the ddb VOS API (ddb_vos.c/ddb_vos.h), with unit tests.ddb_run_csum_check()into the ddb C command set on top ofdv_check_csum(). More details could be found in the draft PR DAOS-17321 ddb: Add checksum check command to ddb C API #18942.csum_checkthrough to the ddb Go CLI, plus documentation. More details could be found in the draft PR DAOS-17321 ddb: Add checksum check command to ddb GO code #18943.Supersedes #18940, which contained all three layers as a single squashed commit.
Changes
dv_check_csum()— new ddb VOS API function (ddb_vos.c,ddb_vos.h)dv_check_csum(daos_handle_t poh, struct dv_tree_path *path, daos_epoch_t epoch, dv_check_csum_cb check_cb, void *cb_arg)mirrors the structure of the existingdv_dump_csum()(open container, build the iod from path, dispatch by value type, close container), but additionally fetches the live data for each checksummed segment, recomputes its checksum(s) with a throwawaydaos_csummer, and compares the result against what was already fetched from disk. Nothing is modified — this is a read-only verification pass.Results are reported through a callback:
got_csumsis an array ofcil->dcl_csum_infos_nrpointers:got_csums[i]holds the checksum recomputed from the currently stored data when entryi's stored checksum did not match, and isNULLwhen it matched (or there was nothing to check) — sogot_csums[i] != NULLdoubles as the per-entry mismatch flag. The function itself returns-DER_CSUMif any entry failed to match, after invoking the callback with the full picture (so callers can report every mismatch, not just the first one).Internally,
check_csum_sv()/check_csum_recx()fetch the checksum metadata via the sameVOS_OF_FETCH_CSUMmechanism as the dump path, thenverify_segment_csum()fetches the live data for that segment (fetch_check_value()), recomputes its checksum, and compares it (daos_csummer_compare_csum_info()), snapshotting a standalone copy of the recomputed checksum (snapshot_csum_info()) on a mismatch so it outlives the throwaway csummer. For array akeys, all overlapping segments are checked independently — a corrupted segment doesn't stop verification of the others, andgot_csums[]correctly isolates which segment(s) failed.As a drive-by cleanup in the same file,
dump_csum_sv()/dump_csum_recx()had theircb_rc/rcdouble-bookkeeping simplified (no behavior change).Test fixtures (
tests/ddb_test_driver.c,tests/ddb_test_driver.h)Adds a third checksum test object (
g_oids[2]) with valid data but a deliberately corrupted stored checksum (single-byte flip applied to an otherwise-correct checksum before writing), for both the single-value and array-akey layouts:csum_test_corrupt_sv_setup()— one corrupted SV.csum_test_corrupt_recx_setup()— an array withDVT_FAKE_RECX_COUNToverlapping segments, onlyDVT_FAKE_RECX_BAD_IDXof which is corrupted, so per-entry isolation can be exercised (the other segment(s) must never be flagged).Tests (
tests/ddb_vos_tests.c)Three new test cases under the existing csum test suite:
check_csum_error_testsvos_cont_openfailure propagationcheck_csum_sv_testsEPOCH_MAX, corrupted csum (-DER_CSUM,got_csums[0]holds the recomputed value), no-callback no-op, callback-error propagationcheck_csum_recx_testsSteps for the author:
After all prior steps are complete: