Skip to content

Commit

Permalink
main, view: Pair view builder drain with its start
Browse files Browse the repository at this point in the history
Before these changes, we only drained the view builder when stopping it,
which was paired with its *construction*. However, since the view
builder depends on group0_service, which is initialized AFTER the view
builder is constructed, that could lead to segmentation faults.
To amend it, we pair draining the view builder with its start, while
leaving the lambda destroying it paired with its construction. This way
we should have a proper initialization order, avoid bugs, and reinforce
our understanding of how Scylla starts up.

Fixes #20772
  • Loading branch information
dawmd committed Dec 12, 2024
1 parent 7c66dc9 commit 94407a8
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 1 deletion.
2 changes: 1 addition & 1 deletion db/view/view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2097,7 +2097,7 @@ future<> view_builder::drain() {

future<> view_builder::stop() {
vlogger.info("Stopping view builder");
return drain();
co_return;
}

view_builder::build_step& view_builder::get_or_create_build_step(table_id base_id) {
Expand Down
3 changes: 3 additions & 0 deletions main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2190,6 +2190,9 @@ To start the scylla server proper, simply invoke as: scylla server (or just scyl
if (cfg->view_building()) {
view_builder.invoke_on_all(&db::view::view_builder::start, std::ref(mm), utils::cross_shard_barrier()).get();
}
auto drain_view_builder = defer_verbose_shutdown("draining view builders", [&] {
view_builder.invoke_on_all(&db::view::view_builder::drain).get();
});

api::set_server_view_builder(ctx, view_builder).get();
auto stop_vb_api = defer_verbose_shutdown("view builder API", [&ctx] {
Expand Down
3 changes: 3 additions & 0 deletions test/lib/cql_test_env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,9 @@ class single_node_cql_env : public cql_test_env {
_view_builder.invoke_on_all([this] (db::view::view_builder& vb) {
return vb.start(_mm.local());
}).get();
auto drain_view_builder = defer([this] {
_view_builder.invoke_on_all(&db::view::view_builder::drain).get();
});

// Create the testing user.
try {
Expand Down

0 comments on commit 94407a8

Please sign in to comment.