Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress ASAN errors on tests that intentially crash the server via crash-memcheck-enabled no #489

Merged
merged 1 commit into from May 12, 2024

Conversation

PingXie
Copy link
Member

@PingXie PingXie commented May 11, 2024

via `crash-memcheck-enabled no`

Signed-off-by: Ping Xie <pingxie@google.com>
@PingXie PingXie requested a review from madolson May 11, 2024 06:51
Copy link

codecov bot commented May 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.81%. Comparing base (4e944ce) to head (303346b).
Report is 14 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable     #489      +/-   ##
============================================
+ Coverage     68.88%   69.81%   +0.92%     
============================================
  Files           109      109              
  Lines         61793    61801       +8     
============================================
+ Hits          42566    43145     +579     
+ Misses        19227    18656     -571     

see 36 files with indirect coverage changes

Copy link
Member

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the failures was exposed by crash-memcheck-enabled?

i remember if it was exposed by crash-memcheck-enabled, it will have some FAST MEMORY TEST log, but in the link, it does not seem to have this log.

@PingXie
Copy link
Member Author

PingXie commented May 11, 2024

does the failures was exposed by crash-memcheck-enabled?

This failure occurred in the new test slot-migration where it simulates primary failures via debug segfault. Since crash-memcheck-enabled is enabled by default, it leads to the execution of doFastMemoryTest.

i remember if it was exposed by crash-memcheck-enabled, it will have some FAST MEMORY TEST log, but in the link, it does not seem to have this log.

I still don't know exactly how to interpret every failure reported but there are quite a few references to doFastMemoryTest w.r.t invalid memory access in the CI log. I don't think there is any harm in disabling memory tests for this one debug segfault case. It will be good to see if it helps with our daily CI failures. WDYT?

==42075== Invalid read of size 8
==42075==    at 0x4852925: memmove (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==42075==    by 0x24095E: memtest_preserving_test (memtest.c:317)
==42075==    by 0x21DA99: memtest_test_linux_anonymous_maps (debug.c:2131)
==42075==    by 0x21DC31: doFastMemoryTest (debug.c:2172)
==42075==    by 0x21E466: printCrashReport (debug.c:2382)
==42075==    by 0x21E154: sigsegvHandler (debug.c:2302)
==42075==    by 0x499051F: ??? (in /usr/lib/x86_64-linux-gnu/libc.so.6)
==42075==    by 0x21941A: debugCommand (debug.c:509)
==42075==    by 0x198FC5: call (server.c:3591)
==42075==    by 0x19AB6B: processCommand (server.c:4227)
==42075==    by 0x1BC9C1: processCommandAndResetClient (networking.c:2492)
==42075==    by 0x1BCC46: processInputBuffer (networking.c:2600)
==42075==  Address 0x4b79000 is in a rwx anonymous segment
==42075== 

Copy link
Member

@madolson madolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only other place we call DEBUG SEGFAULT we also disable this, so we can definitely follow up and try to fix it, but I would like to see if this makes the tests green. They're going to start in an hour, so would rather get this merged and try it out.

@madolson madolson merged commit ac47ca2 into valkey-io:unstable May 12, 2024
17 checks passed
adetunjii pushed a commit to adetunjii/valkey that referenced this pull request May 15, 2024
adetunjii pushed a commit to adetunjii/valkey that referenced this pull request May 15, 2024
…crash-memcheck-enabled no` (valkey-io#489)

Fix daily CI run errors like
https://github.com/valkey-io/valkey/actions/runs/9039450198/job/24842308071#step:6:4176

Signed-off-by: Ping Xie <pingxie@google.com>
Signed-off-by: adetunjii <adetunjithomas1@outlook.com>
adetunjii pushed a commit to adetunjii/valkey that referenced this pull request May 15, 2024
…crash-memcheck-enabled no` (valkey-io#489)

Fix daily CI run errors like
https://github.com/valkey-io/valkey/actions/runs/9039450198/job/24842308071#step:6:4176

Signed-off-by: Ping Xie <pingxie@google.com>
Signed-off-by: adetunjii <adetunjithomas1@outlook.com>
hallmason17 pushed a commit to hallmason17/valkey that referenced this pull request May 15, 2024
hallmason17 pushed a commit to hallmason17/valkey that referenced this pull request May 18, 2024
srgsanky pushed a commit to srgsanky/valkey that referenced this pull request May 19, 2024
adetunjii pushed a commit to adetunjii/valkey that referenced this pull request May 22, 2024
…crash-memcheck-enabled no` (valkey-io#489)

Fix daily CI run errors like
https://github.com/valkey-io/valkey/actions/runs/9039450198/job/24842308071#step:6:4176

Signed-off-by: Ping Xie <pingxie@google.com>
Signed-off-by: Samuel Adetunji <adetunjithomas1@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants