Skip to content

Commit

Permalink
sstables, gdb: Track readers in a linked list
Browse files Browse the repository at this point in the history
For the purpose of scylla-gdb.py command "scylla
active-sstables". Before the patch, readers were located by scanning
the heap for live objects with vtable pointers corresponding to
readers. It was observed that the test scylla_gdb/test_misc.py::test_active_sstables started failing like this:

  gdb.error: Error occurred in Python: Cannot access memory at address 0x300000000000000

This could be explained by there being a live object on the heap which
used to be a reader but now is a different object, and the _sst field
contains some other data which is not a pointer.

To fix, track readers explicitly in a linked list so that the gdb
script can reliably walk readers.

Fixes #18618.
  • Loading branch information
tgrabiec committed May 15, 2024
1 parent fad6c41 commit 4d84451
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
4 changes: 1 addition & 3 deletions scylla-gdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -1797,9 +1797,7 @@ def _lookup_type(type_names):

types = []
try:
# For Scylla < 2.1
# FIXME: this only finds range readers
types = [_lookup_type(['sstable_range_wrapping_reader'])]
return intrusive_list(gdb.parse_and_eval('sstables::_reader_tracker._readers'), link='_tracker_link')
except gdb.error:
try:
# for Scylla <= 4.5
Expand Down
23 changes: 23 additions & 0 deletions sstables/sstable_mutation_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,32 @@
#include "utils/to_string.hh"

#include <boost/range/algorithm/stable_partition.hpp>
#include <boost/intrusive/list.hpp>

namespace sstables {

class reader_tracker {
using list_type = boost::intrusive::list<mp_row_consumer_reader_base,
boost::intrusive::member_hook<mp_row_consumer_reader_base,
mp_row_consumer_reader_base::tracker_link_type,
&mp_row_consumer_reader_base::_tracker_link>,
boost::intrusive::constant_time_size<false>>;
public:
list_type _readers;

void add(mp_row_consumer_reader_base& reader) {
_readers.push_back(reader);
}
};

thread_local reader_tracker _reader_tracker;

mp_row_consumer_reader_base::mp_row_consumer_reader_base(shared_sstable sst)
: _sst(std::move(sst))
{
_reader_tracker.add(*this);
}

atomic_cell make_counter_cell(api::timestamp_type timestamp, fragmented_temporary_buffer::view cell_value) {
static constexpr size_t shard_size = 32;

Expand Down
9 changes: 6 additions & 3 deletions sstables/sstable_mutation_reader.hh
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,14 @@ namespace mx {
}

class mp_row_consumer_reader_base {
public:
using tracker_link_type = bi::list_member_hook<bi::link_mode<bi::auto_unlink>>;
friend class reader_tracker;
protected:
shared_sstable _sst;

tracker_link_type _tracker_link;

// Whether index lower bound is in current partition
bool _index_in_current_partition = false;

Expand All @@ -42,9 +47,7 @@ protected:

std::optional<dht::decorated_key> _current_partition_key;
public:
mp_row_consumer_reader_base(shared_sstable sst)
: _sst(std::move(sst))
{ }
mp_row_consumer_reader_base(shared_sstable sst);

// Called when all fragments relevant to the query range or fast forwarding window
// within the current partition have been pushed.
Expand Down

0 comments on commit 4d84451

Please sign in to comment.