From e683c1367f54eefa68bb0dd7529e89ac156f4a07 Mon Sep 17 00:00:00 2001 From: Duarte Nunes Date: Wed, 11 Jul 2018 21:40:01 +0100 Subject: [PATCH] db/view: Don't have shard 0 clear other shard's status on drop Shard 0 can clear the in-progress build status of all shards when a view finishes building, because we are ensured all writes to the system table have completed with earlier timestamps. This is not the case when dropping a view. A drop can happen concurrently with the build, in which case shard 0 may process the notification before another shard receives it, and before that shard writes to the system table. Fix this by ensuring each shard clears its own status on drop. Signed-off-by: Duarte Nunes --- db/view/view.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/db/view/view.cc b/db/view/view.cc index 8c2940166b..4e790c4a45 100644 --- a/db/view/view.cc +++ b/db/view/view.cc @@ -1279,10 +1279,14 @@ void view_builder::on_drop_view(const sstring& ks_name, const sstring& view_name } })(); if (engine().cpu_id() != 0) { - return make_ready_future(); + // Shard 0 can't remove the entry in the build progress system table on behalf of the + // current shard, since shard 0 may have already processed the notification, and this + // shard may since have updated the system table if the drop happened concurrently + // with the build. + return system_keyspace::remove_view_build_progress(ks_name, view_name); } return when_all_succeed( - system_keyspace::remove_view_build_progress_across_all_shards(ks_name, view_name), + system_keyspace::remove_view_build_progress(ks_name, view_name), system_keyspace::remove_built_view(ks_name, view_name), _sys_dist_ks.remove_view(ks_name, view_name)).handle_exception([ks_name, view_name] (std::exception_ptr ep) { vlogger.warn("Failed to cleanup view {}.{}: {}", ks_name, view_name, ep); @@ -1509,6 +1513,8 @@ future<> view_builder::maybe_mark_view_as_built(view_ptr view, dht::token next_t return seastar::when_all_succeed( system_keyspace::mark_view_as_built(view->ks_name(), view->cf_name()), builder._sys_dist_ks.finish_view_build(view->ks_name(), view->cf_name())).then([view] { + // The view is built, so shard 0 can remove the entry in the build progress system table on + // behalf of all shards. It is guaranteed to have a higher timestamp than the per-shard entries. return system_keyspace::remove_view_build_progress_across_all_shards(view->ks_name(), view->cf_name()); }).then([&builder, view] { auto it = builder._build_notifiers.find(std::pair(view->ks_name(), view->cf_name()));