Raphael Raph Carvalho

Results 183 comments of Raphael Raph Carvalho

since v1: - Added comment explaining how streamer guarantees topology stability - Removed wrong patch: "sstables_loader: Fix stream to happen in token ascending order" - Adjusted tablet_sstable_streamer::stream() as sstables remains...

> * How much of the fix is tablet related (and how much isn't and needs backporting?) this one "sstables_loader: Stream to pending replicas if needed" for example became needed...

since v2: - rebased - simplified [Fix online SSTable loading with concurrent tablet migration](https://github.com/scylladb/scylladb/pull/17449/commits/467289a64273265f893271181744cd4acf1be609) with dht::auto_refreshing_sharder

since v3: - simplified private sstable_directory ctor, by using std::variant - reworked handling of migration during streaming, now taking into account primary_replica_only flag.

The series look good to me. I am very tired today, so tomorrow morning I plan to review the incremental selection path.

just had one comment, other than that, looks good to me.

> CI failures, e.g. https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/6578/artifact/testlog/x86_64/debug/alternator.run.3.log > > ``` > Scylla version 5.5.0~dev-0.20240211.bcfd0238d3ad with build-id b09765cf1edd2abde2301f205f5be244e0a5f188 starting ... > > dht/token.hh:227:44: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be...

> > when accessing out of bound. > > we must have check_tablet_id(id); in get_first_token(). > > I'm fixing that in the next version based on your suggestion to pass...

> The title is a bit misleading, I'd just write in plain words, that we either add or remove tablets' replicas when ALTERing tablets KS. Yes, tablet resize might conflict...

> Seen again in https://jenkins.scylladb.com//job/scylla-master/job/scylla-ci/8285/ (but before the patch that was merged a few hours ago) Seen again in https://jenkins.scylladb.com/job/scylla-master/job/gating-dtest-release-with-consistent-topology-changes/1278/