DAOS-16951 dtx: add invalid records discard capability

[janekmi]

Guide's TLDR

• Introduce a new vos_dtx_discard_invalid() API
• Introduce a few accessory functions validating ILOG, SVT and EVT records
• Make the vos_dtx_discard_invalid() API available via DDB
• New unit test binaries (dtx_ut and ddb_ut)

Guide (full)

• Introduce a new vos_dtx_discard_invalid() API
  • src/include/daos_srv/vos.h - add to the header file
  • src/vos/vos_dtx.c
    • core of the implementation goes here
    • Note: a few types and a helper function from this file have been moved. Please see src/vos/ilog_internal.h for details.
  • unit tests
    • src/dtx/tests/dts_discard_invalid.c - here you find all the unit tests for the vos_dtx_discard_invalid() API
    • src/dtx/tests/SConscript - because of the mocking requirements a new dtx_ut unit test binary has been introduced
    • src/dtx/tests/dtx_ut.c - greatly simplified copy of the dtx_tests.c - the entry point for all unit tests compiled in the binary
    • src/dtx/tests/dts_structs.c - because of the minimal requirements these tests fitted more to the new dtx_ut binary rather than to dtx_tests
• Introduce a few accessory functions validating ILOG, SVT and EVT records
  • ILOG:
    • src/vos/ilog.h - header
    • src/vos/ilog.c - implementation
      • Note: A few things have been moved from here. Please see src/vos/ilog_internal.h for details.
    • src/vos/tests/vts_ilog.c - unit tests
      • src/vos/ilog_internal.h - introduced to easier access ILOG internals for testing purposes.
  • SVT:
    • src/vos/vos_internal.h - header
    • src/vos/vos_tree.c - implementation
    • src/vos/tests/vts_tree.c - unit tests
  • EVT:
    • src/include/daos_srv/evtree.h - header file
    • src/vos/evtree.c - impementation
    • src/vos/tests/vts_evtree.c - unit tests
• Make the vos_dtx_discard_invalid() API available via DDB
  • src/utils/ddb/ddb_commands.c
    • ddb_run_dtx_act_discard_invalid() implementation
    • extend dtx_modify_init() to skip DTX ID initialization if dti_all is requested
    • src/utils/ddb/tests/ddb_commands_tests.c - simple tests
  • src/utils/ddb/ddb.h:
    • add a new command type and a new entry point - ddb_run_dtx_act_discard_invalid()
    • replace struct dtx_act_commit_options and struct dtx_act_abort_options with a single struct dtx_act_options which works for both dtx_act_commit and dtx_act_abort commands as well as the newly introduced dtx_act_discard_invalid
      • src/utils/ddb/tests/ddb_cmd_options_tests.c - update tests accordingly
  • src/utils/ddb/ddb.c:
    • dtx_act_option_parse() instead of dtx_act_commit_option_parse() and dtx_act_abort_option_parse() which works for both old and new dtx_act_* commands
    • connect command parsing and the new entry point
  • dv_dtx_active_entry_discard_invalid() proxy
    • src/utils/ddb/ddb_vos.h and src/utils/ddb/ddb_vos.c
    • src/utils/ddb/tests/ddb_vos_ut.c - unit tests
    • src/utils/ddb/tests/SConscript - because of the mocking requirements a new ddb_ut unit test binary has been introduced
    • src/utils/ddb/tests/ddb_ut.c - greatly simplified copy of the ddb_test_driver.c - the entry point for all unit tests compiled in the binary
• New unit test binaries (dtx_ut and ddb_ut)
  • utils/utest.yaml - add them to standard execution outlets
  • utils/rpms/daos.spec - add them to the RPM package
    • utils/rpms/daos.rpmlintrc - add them as yet another binaries without manuals
  • debian/daos-server-tests.install - add them to the DEB package
    • debian/changelog - report above in the changelog

---

Before requesting gatekeeper:

• Two review approvals and any prior change requests have been resolved.
• Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
• Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
• Commit messages follows the guidelines outlined here.
• Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

• You are the appropriate gatekeeper to be landing the patch.
• The PR has 2 reviews by people familiar with the code, including appropriate owners.
• Githooks were used. If not, request that user install them and check copyright dates.
• Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
• All builds have passed. Check non-required builds for any new compiler warnings.
• Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
• If applicable, the PR has addressed any potential version compatibility issues.
• Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
• Extra checks if forced landing is requested
  • Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
  • No new NLT or valgrind warnings. Check the classic view.
  • Quick-build or Quick-functional is not used.
• Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

[github-actions]

Ticket title is 'DDB: add invalid DTX records discard capability'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-16951

[Nasf-Fan]

The framework and the basic process are fine. The main concern is that for most of cases, the user does not know which DTX entry is corrupted. Means that related DTX ID is known. So for conveniency, the logic itself needs to iterate the active DTX table, and for each active DTX entry, calls ddb_run_dtx_act_discard_invalid().

On the other hand, please check the patch from Jeff #15747, he does similar repair with online mode. Currently, no decision about which one will be used finally, need more discussion.

[janekmi]

The framework and the basic process are fine. The main concern is that for most of cases, the user does not know which DTX entry is corrupted. Means that related DTX ID is known. So for conveniency, the logic itself needs to iterate the active DTX table, and for each active DTX entry, calls ddb_run_dtx_act_discard_invalid().

On the other hand, please check the patch from Jeff #15747, he does similar repair with online mode. Currently, no decision about which one will be used finally, need more discussion.

I added an option to just set all or a specific DTX id in this case.

[Nasf-Fan]

The basic logic for discarding dangling DTX reference is fine. But my concern is that whether all the changes are related with our main purpose of resolving existing dangling DTX reference? We are so closed to the 2.6.3 deadline, I am afraid of landing such a huge patch maybe some challenged, not sure, but if possible, please split unrelated parts into subsequent patch(es). That will be helpful.

[jolivier23]

Sure, my assertion is that we could take one of those fields, rename it to "magic", set a magic number and then allow 0 or magic. We are essentially preventing corruption here so I think using whatever we can for validation is reasonable. @NiuYawei @Nasf-Fan have an opinion here?

[osalyk]

[Nasf-Fan]

We can consider to improve the ability of detection SVT record properly in subsequent patch(es).

[tanabarr]

Go and build changes LGTM