DAOS-16661 common: Integrate PMDK logging system w/ VOS logging system. #14923

grom72 · 2024-08-13T20:28:40Z

pmemobj error and warning messages are written by default to stderr and syslog().
PMDK enables redirecting logging messages to user-specific functions.

All error and warning messages are reported using the DAOS logging function, to ensure a coherent events view for VOS and related pmemobj functions.

Requires:

DAOS-16660 No error message on intentional pmemobj transaction abort pmdk-pkg#43 land to avoid error messages on intentionally canceled transactions.
DAOS-16661 ci: Ignore detail analysis of PMDK warnings in NLT pipeline-lib#457

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:

github-actions · 2024-08-13T20:28:59Z

Ticket title is 'Integrate PMDK logging system w/ VOS logging system.'
Status is 'In Review'
Labels: 'scrubbed_2.8,test_2.8'
https://daosio.atlassian.net/browse/DAOS-16661

daosbuild1 · 2024-08-14T08:35:14Z

Test stage NLT on EL 8.8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-14923/3/testReport/

To enable new message in log in NLT. Allow-unstable-test: true Priority: 2 Required-githooks: true Co-authored-by: Jan Michalski <jan.michalski@intel.com> Co-authored-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com> Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

debian/changelog

To enable new message in log in NLT. Allow-unstable-test: true Priority: 2 Required-githooks: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

Priority: 2 Allow-unstable-test: true Skip-func-hw-test: false Required-githooks: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

Skip-nlt: true Priority: 2 Allow-unstable-test: true Skip-func-hw-test: false Required-githooks: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

brianjmurrell

@grom72 Packaging changes LGTM. Please re-request review from me once you have gotten your two +1s for the core code changes and I will +1 to remove my -1 review. I just don't want my +1 to be misunderstood to be for the core code changes also.

daosbuild1 · 2024-09-04T19:46:05Z

Test stage Functional Hardware Medium UCX Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14923/11/execution/node/1683/log

daosbuild1 · 2024-09-19T19:42:59Z

Test stage Functional Hardware Medium UCX Provider completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-14923/12/execution/node/635/log

Skip-nlt: true Priority: 2 Allow-unstable-test: true Required-githooks: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Allow-unstable-test: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

Static analysis of NLT results does not work properly when a warning source code is out of scope of the project. An error is reported in such case and the whole stage status is changed to FAILURE. ``` [NLT results] [-ERROR-] Can't create fingerprints for some files: [NLT results] [-ERROR-] - 'pmdk/.../src/common/set.c' file not found ... [NLT results] Failing build because analysis result contains errors ``` To prevent this problem, all PMDK-related messages are filtered out. NLT results Warnings section will include warnings but will not provide any detail information about location of this warning in the source code. Based on existing format of nlt-errors.json file the most easy way to filtered all PMDK messages is to use the predefined "pmdk/" preffix in the fileName: ``` ... { "fileName": "pmdk/src/../src/common/set.c", "type": "warning in strict mode", "lineStart": 2796, "description": "warning in strict mode", "message": "util_replica_check() Possible silent data corruption. The unsafe shutdown detection (SDS) is not supported in the pool: /mnt/daos_0/e5737f5f-fa54-45d6-b081-9b2769921610/vos-1\nwarning in strict mode", "severity": "NORMAL" }, ... ``` vos logging system should ensure that the file name of each PMDK related message starts with "pmdk/" prefix. The implementation of the solution is provided in daos-stack/daos#14923 PR Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Allow-unstable-test: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

grom72 · 2025-03-05T19:20:30Z

All PMDK warnings has been muted in NLT and Fault Injection tests as they can not be processed properly.

There is one new PMDK warnings related to non-pmem baked libpmemobj pools.
They were reported to syslog, but now all PMDK errors/warnings are reported in the same place where vos put it's messages to have one coherent log of all pmem related issues.

NLT results analysis reports an error when it can not get source code of the reported warning. PMDK reports warnings/errors via VOS logging system but the source code is not available. Static analysis of NLT results does not work properly when a warning source code is out of scope of the project. An error is reported in such case and the whole stage status is changed to FAILURE. ``` [NLT results] [-ERROR-] Can't create fingerprints for some files: [NLT results] [-ERROR-] - 'pmdk/.../src/common/set.c' file not found ... [NLT results] Failing build because analysis result contains errors ``` To prevent this problem, all PMDK-related messages are filtered out. NLT results Warnings section will include warnings but will not provide any detail information about location of this warning in the source code. Based on existing format of nlt-errors.json file the most easy way to filtered all PMDK messages is to use the predefined "pmdk/" preffix in the fileName: ``` ... { "fileName": "pmdk/src/../src/common/set.c", "type": "warning in strict mode", "lineStart": 2796, "description": "warning in strict mode", "message": "util_replica_check() Possible silent data corruption. The unsafe shutdown detection (SDS) is not supported in the pool: /mnt/daos_0/e5737f5f-fa54-45d6-b081-9b2769921610/vos-1\nwarning in strict mode", "severity": "NORMAL" }, ... ``` The VOS logging system should ensure that the filename of each PMDK-related message starts with a "pmdk/" prefix. The implementation of the solution is provided in daos-stack/daos#14923 PR Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

daos-stack/pipeline-lib#457 landed on master. This change only may affect NLT and Fault Injection tests No need to execute HW and VM Functional Tests Skip-unit-test: true Skip-unit-test-memcheck: true Skip-bullseye: true Skip-fault-injection-test: false Skip-func-test-vm-all: true Skip-func-test-hw: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

daosbuild1 · 2025-03-06T08:00:51Z

Test stage NLT on EL 8.8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-14923/61/testReport/

Reported memchecke issue:

<![CDATA[
{
<insert_a_suppression_name_here>
   Memcheck:Addr4
   fun:__tsan_go_atomic32_fetch_add
   fun:racecall
}
]]>

has already been fixed on master #15967

daos/src/cart/utils/memcheck-cart.supp

Line 735 in 864448e

__tsan_go_atomic32_fetch_add

No other issues in NLT and Fault Injection Tests

grom72 · 2025-03-09T14:59:37Z

@mchaarawi @jolivier23 @NiuYawei do we need one more complete execution of all tests or the actual validation status is good enough?

mchaarawi · 2025-03-10T12:08:45Z

@mchaarawi @jolivier23 @NiuYawei do we need one more complete execution of all tests or the actual validation status is good enough?

I would like to see this PR being green before landing. The history with all that went in is just not easy to stitch together to understand. please merge latest master in and do not skip any tests.
there are also some build failures in gha with clang compiler that should be fixed.

@mchaarawi, @jolivier23
All tests have passed.
https://build.hpdd.intel.com/blue/organizations/jenkins/daos-stack%2Fdaos/detail/PR-14923/62/pipeline

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

jolivier23 · 2025-03-12T14:23:16Z

src/vos/pmdk_log.c

@@ -41,6 +41,29 @@ static void
 pmdk_log_function(enum pmemobj_log_level level, const char *file_name, unsigned line_no,
 		  const char *function_name, const char *message)
 {
+	/* normalize file path - remove leading "../" */
+	while ((*file_name == '.') && (*(file_name + 1) == '.') && (*(file_name + 2) == '/')) {


Hmm, won't this happen even if the log is disabled? Maybe put this in a function and change PMDK_LOG_NOCHECK to a do/while(0) and do this conversion inside the macro?

We can do that, but PMDK produces only error/warning messages which should not be disabled at all.

jolivier23

I think this needs to be updated to put the string conversions inside the macro..unless I'm missing something

Fixed in 7d52d54

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Skip-func-hw-test: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

jolivier23 · 2025-03-12T15:29:23Z

src/vos/pmdk_log.c

+		/* Prefix is needed to filter out PMDK messages in NLT results analysis */         \
+		/* as it is implemented in https://github.com/daos-stack/pipeline-lib/pull/457 */  \
+                                                                                                   \
+		char  file_name_buff[PMDK_LOG_FUNCTION_MAX_FILENAME] = "pmdk/";                    \


minor nit, we usually put declarations at the top of the current block.

Fixed in: 4f2622c

Fix: put declarations at the top of the current block Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Skip-func-hw-test: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

grom72 · 2025-03-13T07:14:51Z

@daos-stack/daos-gatekeeper
Full validation has been done for 7d52d54 - only three ARM builds failed due to missing ARM self-hosted runners. There is a PR #15995 awaiting review that temporarily disables ARM related workflows until we rebuild ARM support in our lab.
The last two commits do not have full validation (Test Hardware excluded), as the latest changes cannot affect those tests in any way. Validation has focused on basic build and functional testing only.

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

daltonbohning · 2025-03-19T16:11:41Z

Unfortunately the latest builds are failing due to a CI issue resolved yesterday. But there is another CI issue #16125 you'll want to wait for too.
And this PR has conflicts now :(

grom72 force-pushed the grom72/pmdk-w-logging branch 3 times, most recently from 75504b7 to ad62bd1 Compare September 3, 2024 07:38

grom72 force-pushed the grom72/pmdk-w-logging branch from ad62bd1 to 149320c Compare September 3, 2024 10:59

grom72 marked this pull request as ready for review September 3, 2024 11:01

grom72 requested review from a team as code owners September 3, 2024 11:01

grom72 requested review from ashleypittman and jolivier23 September 3, 2024 11:15

brianjmurrell previously requested changes Sep 3, 2024

View reviewed changes

debian/changelog Outdated Show resolved Hide resolved

HARK is reported as INFO

1011420

To enable new message in log in NLT. Allow-unstable-test: true Priority: 2 Required-githooks: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

grom72 requested a review from brianjmurrell September 3, 2024 20:56

Fix - force build

f9b926d

Priority: 2 Allow-unstable-test: true Skip-func-hw-test: false Required-githooks: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

grom72 added the release-2.6.1 Targeted for release 2.6.1 label Sep 4, 2024

Skip NLT tests as they blocks functional validation.

45e6548

Skip-nlt: true Priority: 2 Allow-unstable-test: true Skip-func-hw-test: false Required-githooks: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

brianjmurrell reviewed Sep 4, 2024

View reviewed changes

jolivier23 previously approved these changes Sep 5, 2024

View reviewed changes

grom72 added release-2.6.2 Targeted for release 2.6.2 and removed release-2.6.1 Targeted for release 2.6.1 labels Sep 6, 2024

grom72 requested a review from NiuYawei September 16, 2024 14:12

NiuYawei previously approved these changes Sep 18, 2024

View reviewed changes

Fix: apply git-format-clang chanhes

76fe5c6

Skip-nlt: true Priority: 2 Allow-unstable-test: true Required-githooks: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>

Fix: typo and clangformat fixes

1f44161

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Allow-unstable-test: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

Fix clang format issue

0ac1b26

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Allow-unstable-test: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

grom72 requested review from mchaarawi and jolivier23 March 5, 2025 19:20

NiuYawei previously approved these changes Mar 10, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into grom72/pmdk-w-logging

618910b

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

jolivier23 reviewed Mar 12, 2025

View reviewed changes

jolivier23 requested changes Mar 12, 2025

View reviewed changes

Fix: do not process message if PMDK logging is disabled in VOS

7d52d54

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Skip-func-hw-test: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

grom72 dismissed NiuYawei’s stale review via 7d52d54 March 12, 2025 15:08

grom72 requested review from NiuYawei and jolivier23 March 12, 2025 15:10

jolivier23 previously approved these changes Mar 12, 2025

View reviewed changes

DAOS-16661 common: code formating fix

4f2622c

Fix: put declarations at the top of the current block Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Skip-func-hw-test: true Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

grom72 dismissed jolivier23’s stale review via 4f2622c March 12, 2025 15:44

grom72 requested a review from jolivier23 March 12, 2025 15:44

jolivier23 approved these changes Mar 12, 2025

View reviewed changes

NiuYawei approved these changes Mar 13, 2025

View reviewed changes

grom72 requested a review from a team March 13, 2025 07:05

DAOS-16661 common: full validation

e0b37c7

Skip-list: test_ior_intercept_libpil4dfs:DAOS-16260 Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@hpe.com>

daltonbohning removed the request for review from a team March 19, 2025 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-16661 common: Integrate PMDK logging system w/ VOS logging system. #14923

DAOS-16661 common: Integrate PMDK logging system w/ VOS logging system. #14923

grom72 commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

daosbuild1 commented Aug 14, 2024

brianjmurrell left a comment

daosbuild1 commented Sep 4, 2024

daosbuild1 commented Sep 19, 2024

grom72 commented Mar 5, 2025 •

edited

Loading

daosbuild1 commented Mar 6, 2025 •

edited by grom72

Loading

grom72 commented Mar 9, 2025

mchaarawi commented Mar 10, 2025 •

edited by grom72

Loading

jolivier23 Mar 12, 2025

grom72 Mar 12, 2025

jolivier23 left a comment •

edited by grom72

Loading

jolivier23 Mar 12, 2025

grom72 Mar 12, 2025

grom72 commented Mar 13, 2025

daltonbohning commented Mar 19, 2025

DAOS-16661 common: Integrate PMDK logging system w/ VOS logging system. #14923

Are you sure you want to change the base?

DAOS-16661 common: Integrate PMDK logging system w/ VOS logging system. #14923

Conversation

grom72 commented Aug 13, 2024 • edited Loading

Before requesting gatekeeper:

Gatekeeper:

github-actions bot commented Aug 13, 2024 • edited Loading

daosbuild1 commented Aug 14, 2024

brianjmurrell left a comment

Choose a reason for hiding this comment

daosbuild1 commented Sep 4, 2024

daosbuild1 commented Sep 19, 2024

grom72 commented Mar 5, 2025 • edited Loading

daosbuild1 commented Mar 6, 2025 • edited by grom72 Loading

grom72 commented Mar 9, 2025

mchaarawi commented Mar 10, 2025 • edited by grom72 Loading

jolivier23 Mar 12, 2025

Choose a reason for hiding this comment

grom72 Mar 12, 2025

Choose a reason for hiding this comment

jolivier23 left a comment • edited by grom72 Loading

Choose a reason for hiding this comment

jolivier23 Mar 12, 2025

Choose a reason for hiding this comment

grom72 Mar 12, 2025

Choose a reason for hiding this comment

grom72 commented Mar 13, 2025

daltonbohning commented Mar 19, 2025

grom72 commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024 •

edited

Loading

grom72 commented Mar 5, 2025 •

edited

Loading

daosbuild1 commented Mar 6, 2025 •

edited by grom72

Loading

mchaarawi commented Mar 10, 2025 •

edited by grom72

Loading

jolivier23 left a comment •

edited by grom72

Loading