-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Segmentation fault and core dump on alter table cqlstress_lwt_example.blogposts command #10026
Comments
The crash seems to be during view building @eliransin / @psarna please look at this tomorrow and figure out if this is fixed in master or its a change we did in 4.6 and we can revert. If we have no other choice please send a fix - in any case we need one for master (if it was not fixed there) At this point I prefer to revert a change in 4.6 that was not in 4.5 to make progress (if its local and simple) |
I'll take a look now |
How do I download the matching binary? I used Benny's reloc tool (https://github.com/scylladb/scylla_s3_reloc_server) for finding the matching build-id (bf9a6fccf70cbe9f5c482869342c931fadd4c0b2), but it didn't find anything. And without the binary, the core itself is not inspectable. |
Meanwhile, some random thougts.
|
Also, @cvybhu, perhaps you've seen a similar fault lately (one that ended up with a failure in is_satisfied_by)? |
@slivne I always keep getting |
I admit it doea nit allways work for me as well The build that built the packagea and ami is this https://jenkins.scylladb.com/job/scylla-4.6/job/byo/job/byo_build_tests_dtest/9/consoleFull |
The link i provided you does work for me If nithing else works please spin an instance using the ami id in the description its us east1 and yoy should have everything installed on that instance |
The link works when I tunnel from the office ¯\(ツ)/¯ I have no idea why. |
that one is probably it, debuginfo matches, thanks. I hope I'll figure out something meaningful out of it today |
Same / similar situation happened today when running with ============================================================================================ Test:
Issue description ====================================
Coredump:
==================================== Restore Monitor Stack command: Test id: Logs: Jenkins job URL |
Hm, I wonder if it can be due to views and their base tables having mismatched schema versions. @eliransin worked on the code regarding such mismatches, I'll try to dig through git history and see if I find any suspects or missing backports in that area. |
Nope, at least (gdb) p this->_base._raw._id
$5 = {most_sig_bits = 5133691054414107116, least_sig_bits = -7329783304959484384}
(gdb) p this->_view->_raw._base_id
$6 = {most_sig_bits = 5133691054414107116, least_sig_bits = -7329783304959484384} |
but, I see in the schema that the view was created with custom WHERE clause, maybe it's part of the root cause. The WHERE clause is |
I verified via gdb that the restriction that we check against is correct (it represents the |
The final root cause is definitely that we try to operate on |
Ok, I still haven't found the original root cause, but I have some conclusions. Long story short, we try to compare two This function:
These are indeed definition of the same column, but they have different pointers, and index_of from this line: Also, These pointers are different most likely because somehow they come from two schemas - one that was updated lately and one old one perhaps? I have no idea why, but at least we more or less know why the segfault happened. @cvybhu I think that we also need to add a panic-check in this line: if |
Hmm I already went AFK, but the symptom above would definitely happen when the cached "select_statement" from view_info class would not be invalidated when new definition of the base class arrives. @eliransin ideas why/how it could happen? |
If it's a temporary race, throwing when we detect a mismatch described above would fix the issue. I'm not sure however if it's temporary or not, I'll continue the investigation after the weekend |
BTW, we have a fairly easy reproducer for this, so if needs to run with additional assertions or special build to shed more light where it comes from, we should be able to do it quite quickly. |
As observed in scylladb#10026, after schema changes it somehow happened that a column defition that does not match any of the base table columns was passed to expression verification code. The function that looks up the index of a column happens to return -1 when it doesn't find anything, so using this returned index without checking if it's nonnegative results in accessing invalid vector data, and a segfault or silent memory corruption. Therefore, an explicit check is added to see if the column was actually found. This serves two purposes: - avoiding segfaults/memory corruption - making it easier to investigate the root cause of scylladb#10026
@gleb-cloudius suspects this is the same issue #11542 and I suspect it may also be the same issue as #9059 - both those issues involve a view build happening in parallel with altering the base table (even in a trivial way). Piotr Sarna had a pull request for this: #10040 We should go back to this issue and the pull request, and consider merging it (but @gleb-cloudius, did it fix your problem?) |
By this point I am more than suspect that #11542 is the dup of this one. I am sure of it. But the bug is much more severe that what you suggest. There is no race involved. In my reproducer I can change the schema while there is no load running and later when load starts it fails.
Did not check. I can try. |
Yes, it does. So we have a fix for a year but we just forgot about it. I wonder how MV is even used by anyone with the bug. |
@gleb-cloudius the only problem with Piotr's patch was that we didn't know if it solved any bug because we didn't have a good reliable reproducing test for it and Eliran was going to test it but I guess never got around to it. If I understand correctly, today you have an easy to reproduce test for this issue and its fix. Is it a unit/functional test (would have been really nice...) or part of a bigger and slower test? |
I have a bigger and slower reproducer (but 100% one) here #11542 (comment), but I am sure that it is possible to reduce it to one insert and one update. |
@gleb-cloudius I checked that #10040 does not fix #9059, so either #9059 is not a duplicate of this one despite the partial similarity, or #10040 is an incorrect or incomplete fix. |
I tried to create a simpler reproducer than what you have with cassandra-stress, but simpler things seem to work without failing. I'm still trying to find a simpler reproducer, but I guess this bug does not affect everyone using MV but a more specific combination of features. |
On Thu, Nov 03, 2022 at 11:48:37AM -0700, nyh wrote:
> Yes, it does. So we have a fix for a year but we just forgot about it. I wonder how MV is even used by anyone with the bug.
I tried to create a simpler reproducer than what you have with cassandra-stress, but simpler things seem to work without failing. I'm still trying to find a simpler reproducer, but I guess this bug does not affect everyone using MV but a more specific combination of features.
How simple did you try to make it? I did manager to reproduce only with
one select after the population stage. Did you try with LWT? May be it
is required.
…--
Gleb.
|
@gleb-cloudius / @nyh, I'm reading this issue and it's suggested duplicated (#10192, #11542 ) and trying to understand the impact of them. Is it that bad? |
Reproduced with
Installation detailsKernel Version: 5.15.0-1022-aws Scylla Nodes used in this run:
OS / Image: Test: Issue description>>>>>>>
Logs:
|
It is not clear that LWT is needed at all. The bug is in the MV code and LWT test uses views.
Well, it looks like MV is not supposed to be usable at all, but the fact that only this test ever hits it suggest that the bug is in a corner case. Until investigated further by MV people (person) it is hard to tell. May be everyone else are just lucky. |
I wish that other people and not just me cared more about distilling long and slow to minimalistic tests that can reproduce bugs in a fraction of a second and point the finger on exactly which feature is to blame. I'll try to do this today, because a statement like "it's not clear that LWT is needed at all" just means that we haven't finished distilling this test yet - and someone needs to do that. I guess it will have to be me. Also, even without distilling this test, @gleb-cloudius already claims that #10040 fixes it. But as I noted in #10040, the patch as it currently stands breaks other tests, so the patch probably needs to be changed before it goes in. And if we do change it, I'd like to have a test that can check this fix without needing to ask Gleb to run his reproducer for me each time.
@tzach I think @gleb-cloudius is asking you to document that MV is not just "experimental" (like in Cassandra), it is in fact "not supposed to be usable at all". I'm guessing you won't like this statement, so we need to think what needs to be done to make it not true (if it's even true). |
@gleb-cloudius I have a simple cql-pytest reproducer which doesn't need any concurrency or lots of data. Three details of your bigger reproducer ended up being critical for reproducing it in a smaller test:
Note that "view building" is not listed here. It's not needed to reproduce this bug. Normal view updates break just as well. The reproducer: # Reproducer for issue #11542: We have a table with with a materialized
# view with a filter and some data, at which point we modify the base table
# (e.g., add some silly comment) and then try to modify the data.
# The last modification used to fail, logging "Column definition v does not
# match any column in the query selection".
# The same test without the silly base-table modification works, and so does
# the same test without the filter in the materialized view that uses the
# base-regular column v. So does the same test without pre-modification data.
#
# This test is Scylla-only because Cassandra does not support filtering
# on a base-regular column v that is only a key column in the view.
def test_view_update_and_alter_base(cql, test_keyspace, scylla_only):
with new_test_table(cql, test_keyspace, 'p int primary key, v int') as table:
with new_materialized_view(cql, table, '*', 'v, p', 'v >= 0 and p is not null') as mv:
cql.execute(f'INSERT INTO {table} (p,v) VALUES (1,1)')
# In our tests, MV writes are synchronous, so we can read
# immediately
assert len(list(cql.execute(f"SELECT v from {mv}"))) == 1
# Alter the base table, with a silly comment change that doesn't
# change anything important - but still the base schema changes.
cql.execute(f"ALTER TABLE {table} WITH COMMENT = '{unique_name()}'")
# Try to modify an item. This failed in #11542.
cql.execute(f'UPDATE {table} SET v=-1 WHERE p=1')
assert len(list(cql.execute(f"SELECT v from {mv}"))) == 0 |
Great. BTW as I noted here #10026 (comment) no concurrency is needed with the long repro as well. |
Right. Your comment inspired me to dropped my attempts to do things concurrently with the view building, and instead try to figure out what was really important for this failure. Now to fix it (probaby #10040 or something based on it, I need to check why other tests begin to fail). |
When we write to a materialized view, we need to know some information defined in the base table such as the columns in its schema. We have a "view_info" object that tracks each view and its base. This view_info object has a couple of mutable attributes which are used to lazily-calculate and cache the SELECT statement needed to read from the base table. If the base-table schema ever changes - and the code calls set_base_info() at that point - we need to forget this cached statement. If we don't (as before this patch), the SELECT will use the wrong schema and writes will no longer work. This patch also includes a reproducing test that failed before this patch, and passes afterwords. The test creates a base table with a view that has a non-trivial SELECT (it has a filter on one of the base-regular columns), makes a benign modification to the base table (just a silly addition of a comment), and then tries to write to the view - and before this patch it fails. Fixes scylladb#10026 Fixes scylladb#11542
When we write to a materialized view, we need to know some information defined in the base table such as the columns in its schema. We have a "view_info" object that tracks each view and its base. This view_info object has a couple of mutable attributes which are used to lazily-calculate and cache the SELECT statement needed to read from the base table. If the base-table schema ever changes - and the code calls set_base_info() at that point - we need to forget this cached statement. If we don't (as before this patch), the SELECT will use the wrong schema and writes will no longer work. This patch also includes a reproducing test that failed before this patch, and passes afterwords. The test creates a base table with a view that has a non-trivial SELECT (it has a filter on one of the base-regular columns), makes a benign modification to the base table (just a silly addition of a comment), and then tries to write to the view - and before this patch it fails. Fixes #10026 Fixes #11542 (cherry picked from commit 2f2f01b)
When we write to a materialized view, we need to know some information defined in the base table such as the columns in its schema. We have a "view_info" object that tracks each view and its base. This view_info object has a couple of mutable attributes which are used to lazily-calculate and cache the SELECT statement needed to read from the base table. If the base-table schema ever changes - and the code calls set_base_info() at that point - we need to forget this cached statement. If we don't (as before this patch), the SELECT will use the wrong schema and writes will no longer work. This patch also includes a reproducing test that failed before this patch, and passes afterwords. The test creates a base table with a view that has a non-trivial SELECT (it has a filter on one of the base-regular columns), makes a benign modification to the base table (just a silly addition of a comment), and then tries to write to the view - and before this patch it fails. Fixes #10026 Fixes #11542 (cherry picked from commit 2f2f01b)
Backported to 5.0, 5.1. |
Installation details
Kernel version:
5.11.0-1022-aws
Scylla version (or git commit hash):
4.6.rc4-0.20220203.34d470967a0 with build-id bf9a6fccf70cbe9f5c482869342c931fadd4c0b2
Cluster size: 4 nodes (i3.2xlarge)
Scylla running with shards number (live nodes):
longevity-lwt-parallel-24h-4-6-db-node-52f8a7d6-1 (34.205.29.108 | 10.0.3.86): 8 shards
longevity-lwt-parallel-24h-4-6-db-node-52f8a7d6-2 (54.237.170.199 | 10.0.3.210): 8 shards
longevity-lwt-parallel-24h-4-6-db-node-52f8a7d6-3 (54.236.34.38 | 10.0.2.103): 8 shards
longevity-lwt-parallel-24h-4-6-db-node-52f8a7d6-4 (44.197.123.169 | 10.0.2.202): 8 shards
OS (RHEL/CentOS/Ubuntu/AWS AMI):
ami-06d4532db67cc0577
(aws: us-east-1)Test:
longevity-lwt-parallel-schema-changes-with-disruptive-24h-test
Test name:
longevity_lwt_test.LWTLongevityTest.test_lwt_longevity
Test config file(s):
Issue description
====================================
scenario: a Modify-table nemesis ran an alter table command of:
ALTER TABLE cqlstress_lwt_example.blogposts WITH min_index_interval = 256;
Then the node got a segmentation fault and a core dump:
might also be related to #9933
core dump details:
====================================
Restore Monitor Stack command:
$ hydra investigate show-monitor 52f8a7d6-6b64-4051-9232-7bb17ee37cf3
Restore monitor on AWS instance using Jenkins job
Show all stored logs command:
$ hydra investigate show-logs 52f8a7d6-6b64-4051-9232-7bb17ee37cf3
Test id:
52f8a7d6-6b64-4051-9232-7bb17ee37cf3
Logs:
grafana - [https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_140655/grafana-screenshot-longevity-lwt-parallel-schema-changes-with-disruptive-24h-test-scylla-per-server-metrics-nemesis-20220203_140822-longevity-lwt-parallel-24h-4-6-monitor-node-52f8a7d6-1.png](https://www.google.com/url?q=https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_140655/grafana-screenshot-longevity-lwt-parallel-schema-changes-with-disruptive-24h-test-scylla-per-server-metrics-nemesis-20220203_140822-longevity-lwt-parallel-24h-4-6-monitor-node-52f8a7d6-1.png%255D(https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_140655/grafana-screenshot-longevity-lwt-parallel-schema-changes-with-disruptive-24h-test-scylla-per-server-metrics-nemesis-20220203_140822-longevity-lwt-parallel-24h-4-6-monitor-node-52f8a7d6-1.png)&source=gmail-html&ust=1643991989961000&usg=AOvVaw2bEFl2MuSVYjDw3gGavtuJ)
grafana - [https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_140655/grafana-screenshot-overview-20220203_140655-longevity-lwt-parallel-24h-4-6-monitor-node-52f8a7d6-1.png](https://www.google.com/url?q=https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_140655/grafana-screenshot-overview-20220203_140655-longevity-lwt-parallel-24h-4-6-monitor-node-52f8a7d6-1.png%255D(https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_140655/grafana-screenshot-overview-20220203_140655-longevity-lwt-parallel-24h-4-6-monitor-node-52f8a7d6-1.png)&source=gmail-html&ust=1643991989961000&usg=AOvVaw13HqeHmXOzsHnNzJNlm2GO)
db-cluster - [https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/db-cluster-52f8a7d6.tar.gz](https://www.google.com/url?q=https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/db-cluster-52f8a7d6.tar.gz%255D(https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/db-cluster-52f8a7d6.tar.gz)&source=gmail-html&ust=1643991989961000&usg=AOvVaw2kJrwh0F_8pQaljHPY7rPT)
loader-set - [https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/loader-set-52f8a7d6.tar.gz](https://www.google.com/url?q=https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/loader-set-52f8a7d6.tar.gz%255D(https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/loader-set-52f8a7d6.tar.gz)&source=gmail-html&ust=1643991989961000&usg=AOvVaw2eNs_9NpU_wMncVPj-6_Du)
monitor-set - [https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/monitor-set-52f8a7d6.tar.gz](https://www.google.com/url?q=https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/monitor-set-52f8a7d6.tar.gz%255D(https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/monitor-set-52f8a7d6.tar.gz)&source=gmail-html&ust=1643991989961000&usg=AOvVaw1yeaHnNHISpMfeVwIxmtAD)
sct - [https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/sct-runner-52f8a7d6.tar.gz](https://www.google.com/url?q=https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/sct-runner-52f8a7d6.tar.gz%255D(https://cloudius-jenkins-test.s3.amazonaws.com/52f8a7d6-6b64-4051-9232-7bb17ee37cf3/20220203_141729/sct-runner-52f8a7d6.tar.gz)&source=gmail-html&ust=1643991989961000&usg=AOvVaw2nGemjF43q8HSdpfW_znme)
Jenkins job URL
The text was updated successfully, but these errors were encountered: