view,table: fix waiting for view updates during building

View updates sent as part of the view building process should never
be ignored, but fd49fd7 introduced a bug which may cause exactly that:
the updates are mistakenly sent to background, so the view builder
will not receive negative feedback if an update failed, which will
in turn not cause a retry. Consequently, view building may report
that it "finished" building a view, while some of the updates were
lost. A simple fix is to restore previous behaviour - all updates
triggered by view building are now waited for.

Fixes #6038
Tests: unit(dev),
dtest: interrupt_build_process_with_resharding_low_to_half_test
This commit is contained in:
Piotr Sarna
2020-03-18 10:26:42 +01:00
committed by Nadav Har'El
parent 62c34a9085
commit 0c11e07faf
3 changed files with 24 additions and 10 deletions

View File

@@ -1073,7 +1073,8 @@ future<> mutate_MV(
db::view::stats& stats,
cf_stats& cf_stats,
db::timeout_semaphore_units pending_view_updates,
service::allow_hints allow_hints)
service::allow_hints allow_hints,
wait_for_all_updates wait_for_all)
{
auto fs = std::make_unique<std::vector<future<>>>();
fs->reserve(view_updates.size());
@@ -1152,9 +1153,13 @@ future<> mutate_MV(
maybe_account_failure = std::move(maybe_account_failure)] (future<>&& f) mutable {
return maybe_account_failure(std::move(f), std::move(*paired_endpoint), is_endpoint_local, updates_pushed_remote);
});
// The update is sent to background in order to preserve availability,
// its parallelism is limited by view_update_concurrency_semaphore
(void)view_update;
if (wait_for_all) {
fs->push_back(std::move(view_update));
} else {
// The update is sent to background in order to preserve availability,
// its parallelism is limited by view_update_concurrency_semaphore
(void)view_update;
}
}
} else if (!pending_endpoints.empty()) {
// If there is no paired endpoint, it means there's a range movement going on (decommission or move),
@@ -1182,9 +1187,13 @@ future<> mutate_MV(
maybe_account_failure = std::move(maybe_account_failure)] (future<>&& f) {
return maybe_account_failure(std::move(f), std::move(target), false, updates_pushed_remote);
});
// The update is sent to background in order to preserve availability,
// its parallelism is limited by view_update_concurrency_semaphore
(void)view_update;
if (wait_for_all) {
fs->push_back(std::move(view_update));
} else {
// The update is sent to background in order to preserve availability,
// its parallelism is limited by view_update_concurrency_semaphore
(void)view_update;
}
}
}
auto f = seastar::when_all_succeed(fs->begin(), fs->end());

View File

@@ -116,13 +116,16 @@ query::clustering_row_ranges calculate_affected_clustering_ranges(
const mutation_partition& mp,
const std::vector<view_ptr>& views);
struct wait_for_all_updates_tag {};
using wait_for_all_updates = bool_class<wait_for_all_updates_tag>;
future<> mutate_MV(
const dht::token& base_token,
std::vector<frozen_mutation_and_schema> view_updates,
db::view::stats& stats,
cf_stats& cf_stats,
db::timeout_semaphore_units pending_view_updates,
service::allow_hints allow_hints);
service::allow_hints allow_hints,
wait_for_all_updates wait_for_all);
/**
* create_virtual_column() adds a "virtual column" to a schema builder.