[MDEV-23510] Crash in my_strnncoll_binary while running point-select Created: 2020-08-19 Updated: 2021-11-30 Resolved: 2021-02-25 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Locking, Server, Storage Engine - InnoDB |
| Affects Version/s: | 10.5, 10.6 |
| Fix Version/s: | 10.2.38, 10.3.29, 10.4.19, 10.5.10 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Krunal Bauskar | Assignee: | Daniel Black |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | race | ||
| Environment: |
MDBonARM, arm64 |
||
| Issue Links: |
|
||||||||||||||||||||
| Description |
----------------
|
| Comments |
| Comment by Krunal Bauskar [ 2020-09-04 ] | ||||||||||||||||||||
| ||||||||||||||||||||
| Comment by Elena Stepanova [ 2020-10-18 ] | ||||||||||||||||||||
|
In addition to Is it a production server? If not, could you maybe get a stack trace from an instrumented build, e.g. with ASAN-debug? It could help to match it more precisely with other issues. | ||||||||||||||||||||
| Comment by Krunal Bauskar [ 2021-01-13 ] | ||||||||||||||||||||
|
The said issue is now reproduced easily using unit.lf (unit-test meant for lf-hash) on ARM server. Disabling the single handler should help us get the core-dump. Reason looks quite same as the one posted earlier. May be expert in lf-hash can take a look or suggest what else could be looked into.
| ||||||||||||||||||||
| Comment by Marko Mäkelä [ 2021-01-13 ] | ||||||||||||||||||||
|
We also have the unit.lf test failing on buildbot.mariadb.org on aarch64-ubuntu-1804:
| ||||||||||||||||||||
| Comment by Marko Mäkelä [ 2021-01-20 ] | ||||||||||||||||||||
|
wlad, the reported stack trace occurs in MDL and you are the maintainer of that area. Apart from MDL (since I think that we should consider replacing the lf_hash implementation with some regularly maintained library, possibly based on C++ std::atomic. | ||||||||||||||||||||
| Comment by Sergei Golubchik [ 2021-01-20 ] | ||||||||||||||||||||
|
might be a lf-hash issue on arm? | ||||||||||||||||||||
| Comment by Elena Stepanova [ 2021-01-20 ] | ||||||||||||||||||||
|
Taking into account (at least) the stack trace in MDEV-23895, 4th from the top, would you still think it's an arm-specific issue? | ||||||||||||||||||||
| Comment by Vladislav Vaintroub [ 2021-01-20 ] | ||||||||||||||||||||
|
The pure unit test, not poisoned by any Item_func fails, so the problem is in lf_hash and multithreading, and ARM. | ||||||||||||||||||||
| Comment by Vladislav Vaintroub [ 2021-01-20 ] | ||||||||||||||||||||
|
As for concurrent "not invented here" libraries implementing concurrent hash maps, there is something I just googled up The author seems to have a good understanding of concurrency and CPU models etc. | ||||||||||||||||||||
| Comment by Sergei Golubchik [ 2021-01-25 ] | ||||||||||||||||||||
|
Doesn't fail on buildbot.askmonty.org. Using aarch64-ubuntu-1804 VM on an aarch64 ubuntu host. I need some way to repeat the issue. | ||||||||||||||||||||
| Comment by Krunal Bauskar [ 2021-01-27 ] | ||||||||||||||||||||
|
Sergei, Reproducing failure has been challenging. Fortunately, recently unit-test started failing for me on my 24 cores vm when run repeatedly. I tried to reduce it by lowering the scalability but then test-case started to pass continuously. Maybe you can also try to play around with it to see if some configuration can help reproduce the crash with the said VM. #define CYCLES 3000 | ||||||||||||||||||||
| Comment by Marko Mäkelä [ 2021-02-12 ] | ||||||||||||||||||||
|
Another SIGSEGV on aarch64-rhel-8:
| ||||||||||||||||||||
| Comment by Daniel Black [ 2021-02-15 ] | ||||||||||||||||||||
|
I had a theory that a SEGV would be caused by a read of a memory address that was only partially written. Normally we assume writes are atomic but they don't need to be. The l_find uses the LF_SLIST structure for searching. there are a number of 64 pointer types with a uint32 in the middle. This breaks up the alignment. The order isn't necessary so uint32 goes at the end. https://github.com/MariaDB/server/commit/d30c1331a18d875e553f3fcf544997e4f33fb943 - branch bb-10.5-danielblack- | ||||||||||||||||||||
| Comment by Krunal Bauskar [ 2021-02-15 ] | ||||||||||||||||||||
|
I have tested the fix suggested by Daniel and it is working fine when run with the said unit-test-case. | ||||||||||||||||||||
| Comment by Daniel Black [ 2021-02-21 ] | ||||||||||||||||||||
|
Ok, ready. I do remember hitting a lf_hash thing in ppc64le but never got to the bottom of this. bb-10.2-danielblack-
For 10.5 - bb-10.5-danielblack-
| ||||||||||||||||||||
| Comment by Marko Mäkelä [ 2021-02-24 ] | ||||||||||||||||||||
|
Thank you. I think that I am really only entitled to review the InnoDB change (using the common wrapper my_malloc_aligned()). That change looked OK to me. As far as I understand, the necessary change is the first one, which to my understanding passed tests on the affected platform. I did not completely understand the issue, though. If some data must or must not be in the same cache line with something else, then we probably would want to allocate aligned memory, passing the cache line size as the parameter. I did not see that happening. You passed an alignment constraint of only sizeof(void*), which I think should be trivially satisfied by any malloc() implementation. The implementation of my_malloc_aligned() looks wrong to me in one aspect: It allows a fall-back to unaligned malloc(). We do not want to reintroduce | ||||||||||||||||||||
| Comment by Daniel Black [ 2021-02-25 ] | ||||||||||||||||||||
|
Pushed uncontroversial first commit. The entire `uchar *key` must be within a cache line to be able to atomicly read it by a CPU. So with 32bits before and after the cache line boundary, one CPU could be writing an upper 1/2 of the address while another CPU is reading the lower 1/2. Eventually the writer CPU will finish, however the reader combining it with the other 1/2 is an invalid address. leaving other commits for a separate MDEV later: |