assertion failure: ./utils/intrusive_btree.hh:695: intrusive_b::tree<rows_entry, &rows_entry::_link, rows_entry::tri_compare, 12, 20, intrusive_b::key_search::linear, intrusive_b::with_debug::no>::iterator_base<false>::iterator_base(intrusive_b::tree::iterator_base::key_hook_ptr, intrusive_b::key_index) [Key = rows_entry, Hook = &rows_entry::_link, Compare = rows_entry::tri_compare, NodeSize = 12, LinearThreshold = 20, Search = intrusive_b::key_search::linear, Debug = intrusive_b::with_debug::no, Const = false]: Assertion `h->attached()' failed. #9728
Comments
avikivity
changed the title
|
Decoded (hopefully correctly) |
|
IOW -- there's a |
|
Is this some race condition? It seems odd I ran into this during a pretty simple write test, and ran into it not 100% of the time. |
|
That's really strange. It looks more like mishandling of exception or misordered insertion/removal of a entry wrt yields. |
|
@jmaixl can you share the schema? maybe it will give us a clue. |
xemul
added a commit
to xemul/scylla
that referenced
this issue
Dec 8, 2021
The B-tree's insert_before() is throwing operation, its caller must account for that. When the rows_entry's collection was switched on B-tree all the risky places were fixed by ee9e104, but few places went under the radar. In the cache_flat_mutation_reader there's a place where a C-pointer is inserted into the tree, thus potentially leaking the entry. In the partition_snapshot_row_cursor there are two places that not only leak the entry, but also leave it in the LRU list. The latter it quite nasty, because those entry can be evicted, eviction code tries to get rows_entry iterator from "this", but the hook happens to be unattached (because insertion threw) and fails the assert. fixes: scylladb#9728 Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
tgrabiec
added a commit
that referenced
this issue
Dec 8, 2021
There are several places that (still) use throwing b-tree .insert_before() method and don't manage the inserted object lifetime. Some of those places also leave the leaked rows_entry on the LRU delaying the assertion failure by the time those entries get evicted (#9728) To prevent such surprises in the future, the set removes the non-safe inserters from the B-tree code. Actually most of this set is that removal plus preparations for reviewability. * xemul/br-rows-insertion-exception-safety: btree: Earnestly discourage from insertion of plain references row-cache: Handle exception (un)safety of rows_entry insertion partition_snapshot_row_cursor: Shuffle ensure_result creation mutation_partition: Use B-tree insertion sugar tests: Make B-tree tests use unique-ptrs for insertion
Here is the schema: About 8 non-primary key columns are left unset if that makes a difference. |
tgrabiec
added a commit
that referenced
this issue
Dec 10, 2021
There are several places that (still) use throwing b-tree .insert_before() method and don't manage the inserted object lifetime. Some of those places also leave the leaked rows_entry on the LRU delaying the assertion failure by the time those entries get evicted (#9728) To prevent such surprises in the future, the set removes the non-safe inserters from the B-tree code. Actually most of this set is that removal plus preparations for reviewability. * xemul/br-rows-insertion-exception-safety-2: btree: Earnestly discourage from insertion of plain references row-cache: Handle exception (un)safety of rows_entry insertion partition_snapshot_row_cursor: Shuffle ensure_result creation mutation_partition: Use B-tree insertion sugar tests: Make B-tree tests use unique-ptrs for insertion
|
Just wondering, how soon should I expect this fix to be backported to 4.5 and released? (or perhaps in general how often do these bugfix releases come out or is there a timeline available somewhere?) |
|
About a week. |
avikivity
pushed a commit
that referenced
this issue
Dec 14, 2021
Both places get the C-pointer on the freshly allocated rows_entry, insert it where needed and return back the dereferenced pointer. The C-pointer is going to become smart-pointer that would go out of scope before return. This change prepares for that by constructing the ensure_result from the iterator, that's returned from insertion of the entry. Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> (cherry picked from commit 9fd8db3) Ref #9728
avikivity
pushed a commit
that referenced
this issue
Dec 14, 2021
The B-tree's insert_before() is throwing operation, its caller must account for that. When the rows_entry's collection was switched on B-tree all the risky places were fixed by ee9e104, but few places went under the radar. In the cache_flat_mutation_reader there's a place where a C-pointer is inserted into the tree, thus potentially leaking the entry. In the partition_snapshot_row_cursor there are two places that not only leak the entry, but also leave it in the LRU list. The latter it quite nasty, because those entry can be evicted, eviction code tries to get rows_entry iterator from "this", but the hook happens to be unattached (because insertion threw) and fails the assert. fixes: #9728 Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> (cherry picked from commit ee10363)
avikivity
pushed a commit
that referenced
this issue
Dec 14, 2021
Both places get the C-pointer on the freshly allocated rows_entry, insert it where needed and return back the dereferenced pointer. The C-pointer is going to become smart-pointer that would go out of scope before return. This change prepares for that by constructing the ensure_result from the iterator, that's returned from insertion of the entry. Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> (cherry picked from commit 9fd8db3) Ref #9728
avikivity
pushed a commit
that referenced
this issue
Dec 14, 2021
The B-tree's insert_before() is throwing operation, its caller must account for that. When the rows_entry's collection was switched on B-tree all the risky places were fixed by ee9e104, but few places went under the radar. In the cache_flat_mutation_reader there's a place where a C-pointer is inserted into the tree, thus potentially leaking the entry. In the partition_snapshot_row_cursor there are two places that not only leak the entry, but also leave it in the LRU list. The latter it quite nasty, because those entry can be evicted, eviction code tries to get rows_entry iterator from "this", but the hook happens to be unattached (because insertion threw) and fails the assert. fixes: #9728 Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> (cherry picked from commit ee10363)
avikivity
pushed a commit
that referenced
this issue
Dec 14, 2021
Both places get the C-pointer on the freshly allocated rows_entry, insert it where needed and return back the dereferenced pointer. The C-pointer is going to become smart-pointer that would go out of scope before return. This change prepares for that by constructing the ensure_result from the iterator, that's returned from insertion of the entry. Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> (cherry picked from commit 9fd8db3) Ref #9728
avikivity
pushed a commit
that referenced
this issue
Dec 14, 2021
The B-tree's insert_before() is throwing operation, its caller must account for that. When the rows_entry's collection was switched on B-tree all the risky places were fixed by ee9e104, but few places went under the radar. In the cache_flat_mutation_reader there's a place where a C-pointer is inserted into the tree, thus potentially leaking the entry. In the partition_snapshot_row_cursor there are two places that not only leak the entry, but also leave it in the LRU list. The latter it quite nasty, because those entry can be evicted, eviction code tries to get rows_entry iterator from "this", but the hook happens to be unattached (because insertion threw) and fails the assert. fixes: #9728 Signed-off-by: Pavel Emelyanov <xemul@scylladb.com> (cherry picked from commit ee10363)
|
Backported to 4.4, 4.5, 4.6. |
|
I un-backported from 4.4 (fails to build). @xemul please do the backport, you can start from 4.5 where I resolved some conflicts. |
|
@avikivity , 4.4 doesn't need it. |
|
Even better! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Installation details
Scylla version (or git commit hash): 4.5.1-0.20211024.4c0eac049
Cluster size: 3
OS (RHEL/CentOS/Ubuntu/AWS AMI): Centos 7.9.2009
Hardware details (for performance issues)
Platform (physical/VM/cloud instance type/docker): AWS c5ad.4xlarge
Report UUID containing troubleshooting files: fdfa9238-7a6f-4332-8b79-ca7d4f987a13
I ran a write test against a 3 node cluster, inserting 30m rows at 25k rows/second.
The table has 1 partition key column and 3 clustering columns, with each partition containing 10k rows. The table uses LeveledCompactionStrategy.
What happens is that during some runs of this test, 1 or 2 of the nodes will suddenly go down and restart. On the down/restarted node, I see this in the log:
Any idea of what this means?
The text was updated successfully, but these errors were encountered: