[PATCH 0/2] Allow reshape to be aborted

0 views
Skip to first unread message

Raphael S. Carvalho

unread,
Jul 28, 2021, 10:47:06 AMJul 28
to scylla...@googlegroups.com, Raphael S. Carvalho
Now reshape can be aborted on either boot or refresh.

The workflow is:
1) reshape starts
2) user notices it's taking too long
3) nodetool stop RESHAPE

the good thing is that completed reshape work isn't lost, allowing
table to enjoy the benefits of all reshaping done up to the abortion
point.

Fixes #7738.

Raphael S. Carvalho (2):
api: make compaction manager api available earlier
compaction: Allow reshape to be aborted

api/api_init.hh | 1 +
api/api.cc | 9 ++++++++-
compaction/compaction_manager.cc | 1 +
main.cc | 3 +++
sstables/sstable_directory.cc | 8 +++++++-
5 files changed, 20 insertions(+), 2 deletions(-)

--
2.31.1

Raphael S. Carvalho

unread,
Jul 28, 2021, 10:47:07 AMJul 28
to scylla...@googlegroups.com, Raphael S. Carvalho
That will be needed for aborting reshape on boot.

Refs #7738.

Signed-off-by: Raphael S. Carvalho <raph...@scylladb.com>
---
api/api_init.hh | 1 +
api/api.cc | 9 ++++++++-
main.cc | 3 +++
3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/api/api_init.hh b/api/api_init.hh
index ecba22f17..8dce1760a 100644
--- a/api/api_init.hh
+++ b/api/api_init.hh
@@ -86,6 +86,7 @@ future<> set_server_storage_proxy(http_context& ctx);
future<> set_server_stream_manager(http_context& ctx);
future<> set_server_gossip_settle(http_context& ctx);
future<> set_server_cache(http_context& ctx);
+future<> set_server_compaction_manager(http_context& ctx);
future<> set_server_done(http_context& ctx);

}
diff --git a/api/api.cc b/api/api.cc
index bef2a736e..523ada75e 100644
--- a/api/api.cc
+++ b/api/api.cc
@@ -178,13 +178,20 @@ future<> set_server_gossip_settle(http_context& ctx) {
});
}

-future<> set_server_done(http_context& ctx) {
+future<> set_server_compaction_manager(http_context& ctx) {
auto rb = std::make_shared < api_registry_builder > (ctx.api_doc);

return ctx.http_server.set_routes([rb, &ctx](routes& r) {
rb->register_function(r, "compaction_manager",
"The Compaction manager API");
set_compaction_manager(ctx, r);
+ });
+}
+
+future<> set_server_done(http_context& ctx) {
+ auto rb = std::make_shared < api_registry_builder > (ctx.api_doc);
+
+ return ctx.http_server.set_routes([rb, &ctx](routes& r) {
rb->register_function(r, "lsa", "Log-structured allocator API");
set_lsa(ctx, r);

diff --git a/main.cc b/main.cc
index 9c102639f..364b6292c 100644
--- a/main.cc
+++ b/main.cc
@@ -1029,6 +1029,9 @@ int main(int ac, char** av) {

distributed_loader::ensure_system_table_directories(db).get();

+ // making compaction manager api available, after system keyspace has already been established.
+ api::set_server_compaction_manager(ctx).get();
+
supervisor::notify("loading non-system sstables");
distributed_loader::init_non_system_keyspaces(db, proxy, mm).get();

--
2.31.1

Raphael S. Carvalho

unread,
Jul 28, 2021, 10:47:09 AMJul 28
to scylla...@googlegroups.com, Raphael S. Carvalho
Now reshape can be aborted on either boot or refresh.

The workflow is:
1) reshape starts
2) user notices it's taking too long
3) nodetool stop RESHAPE

the good thing is that completed reshape work isn't lost, allowing
table to enjoy the benefits of all reshaping done up to the abortion
point.

Fixes #7738.

Signed-off-by: Raphael S. Carvalho <raph...@scylladb.com>
---
compaction/compaction_manager.cc | 1 +
sstables/sstable_directory.cc | 8 +++++++-
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/compaction/compaction_manager.cc b/compaction/compaction_manager.cc
index 7f33d6e20..c3b80f2ef 100644
--- a/compaction/compaction_manager.cc
+++ b/compaction/compaction_manager.cc
@@ -317,6 +317,7 @@ future<> compaction_manager::run_custom_job(column_family* cf, sstables::compact
f.get();
} catch (sstables::compaction_stop_exception& e) {
cmlog.info("{} was abruptly stopped, reason: {}", task->type, e.what());
+ throw;
} catch (...) {
cmlog.error("{} failed: {}", task->type, std::current_exception());
throw;
diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc
index 08a67e932..4d2a5b381 100644
--- a/sstables/sstable_directory.cc
+++ b/sstables/sstable_directory.cc
@@ -362,7 +362,13 @@ future<uint64_t> sstable_directory::reshape(compaction_manager& cm, table& table
return collect_output_sstables_from_reshaping(std::move(new_sstables));
});
});
- }).then([] {
+ }).then_wrapped([&table] (future<> f) {
+ try {
+ f.get();
+ } catch (sstables::compaction_stop_exception& e) {
+ dirlog.info("Table {}.{} with compaction strategy {} had reshape successfully aborted.", table.schema()->ks_name(), table.schema()->cf_name(), table.get_compaction_strategy().name());
+ return make_ready_future<stop_iteration>(stop_iteration::yes);
+ }
return make_ready_future<stop_iteration>(stop_iteration::no);
});
}).then([&reshaped_size] {
--
2.31.1

Benny Halevy

unread,
Jul 28, 2021, 11:10:17 AMJul 28
to Raphael S. Carvalho, scylla...@googlegroups.com
LGTM

Raphael S. Carvalho

unread,
Jul 29, 2021, 3:53:07 PMJul 29
to Benny Halevy, scylladb-dev, Maintainers

Avi Kivity

unread,
Aug 1, 2021, 2:53:09 PMAug 1
to Raphael S. Carvalho, scylla...@googlegroups.com

On 28/07/2021 17.46, Raphael S. Carvalho wrote:
> Now reshape can be aborted on either boot or refresh.
>
> The workflow is:
> 1) reshape starts
> 2) user notices it's taking too long
> 3) nodetool stop RESHAPE
>
> the good thing is that completed reshape work isn't lost, allowing
> table to enjoy the benefits of all reshaping done up to the abortion
> point.
>
> Fixes #7738.
>
> Signed-off-by: Raphael S. Carvalho <raph...@scylladb.com>
> ---
> compaction/compaction_manager.cc | 1 +
> sstables/sstable_directory.cc | 8 +++++++-
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/compaction/compaction_manager.cc b/compaction/compaction_manager.cc
> index 7f33d6e20..c3b80f2ef 100644
> --- a/compaction/compaction_manager.cc
> +++ b/compaction/compaction_manager.cc
> @@ -317,6 +317,7 @@ future<> compaction_manager::run_custom_job(column_family* cf, sstables::compact
> f.get();
> } catch (sstables::compaction_stop_exception& e) {
> cmlog.info("{} was abruptly stopped, reason: {}", task->type, e.what());
> + throw;


How does this throw affect non-reshape compaction?

Raphael S. Carvalho

unread,
Aug 1, 2021, 10:01:32 PMAug 1
to Avi Kivity, scylladb-dev
The other user is resharding. Resharding can be stopped[1], only on
refresh (that's because compaction api isn't available in early boot
phase until this series), after 6ff8bb4eac662d, and when that happens,
manager fails to propagate exception upwards, and table won't allow a
shared sstable to even reach the sstable set, as expected
(https://github.com/scylladb/scylla/blob/next/table.cc#L341). From now
on, manager will propagate the exception and the difference is that
refresh will fail with the stop exception, so the user will have to
rerun the command.

[1]: nodetool stop cannot be used for that though as RESHARD isn't a
recognized type, so one would have to explicitly use HTTP api to
achieve that. Along with this series, nodetool was patched to
recognize the RESHAPE type.

Avi Kivity

unread,
Aug 2, 2021, 5:31:55 AMAug 2
to Raphael S. Carvalho, scylladb-dev
Can't it happen to any compaction?


> Resharding can be stopped[1], only on
> refresh (that's because compaction api isn't available in early boot
> phase until this series), after 6ff8bb4eac662d, and when that happens,
> manager fails to propagate exception upwards, and table won't allow a
> shared sstable to even reach the sstable set, as expected
> (https://github.com/scylladb/scylla/blob/next/table.cc#L341). From now
> on, manager will propagate the exception and the difference is that
> refresh will fail with the stop exception, so the user will have to
> rerun the command.


What happens if an ordinary compaction is stopped like that?


Propagating the exception seems more correct than not, but I'd like to
understand if there was a reason to swallow the exception originally.

Raphael S. Carvalho

unread,
Aug 2, 2021, 9:07:15 AMAug 2
to Avi Kivity, scylladb-dev
no, regular, cleanup, go through their own specialized functions and
they share this common handler
https://github.com/scylladb/scylla/blob/next/compaction/compaction_manager.cc#L525.
run_custom_job() is limited to external users like resharding and
reshape, which can specify a custom job to run through the manager. In
my opinion, this is all a bit messy and I want to refactor exception
handling there soon.

>
>
> > Resharding can be stopped[1], only on
> > refresh (that's because compaction api isn't available in early boot
> > phase until this series), after 6ff8bb4eac662d, and when that happens,
> > manager fails to propagate exception upwards, and table won't allow a
> > shared sstable to even reach the sstable set, as expected
> > (https://github.com/scylladb/scylla/blob/next/table.cc#L341). From now
> > on, manager will propagate the exception and the difference is that
> > refresh will fail with the stop exception, so the user will have to
> > rerun the command.
>
>
> What happens if an ordinary compaction is stopped like that?

compaction is retried if stopped using exponential backoff retry
logic, not necessarily with the same set of candidates though. a new
input set is picked from strategy when the time for retry comes.

>
>
> Propagating the exception seems more correct than not, but I'd like to
> understand if there was a reason to swallow the exception originally.

Probably a bug. Initially only asynchronous resharding was using it,
so it wasn't an issue. System could also work with shared sstables.
Now with synchronous resharding and reshape, propagating the stop
exception is the right thing to do.

Avi Kivity

unread,
Aug 2, 2021, 10:17:23 AMAug 2
to Raphael S. Carvalho, scylla...@googlegroups.com
Please post a git url.

On 28/07/2021 17.46, Raphael S. Carvalho wrote:

Raphael S. Carvalho

unread,
Aug 2, 2021, 1:03:14 PMAug 2
to Avi Kivity, scylladb-dev
On Mon, Aug 2, 2021 at 11:17 AM Avi Kivity <a...@scylladb.com> wrote:
>
> Please post a git url.

https://github.com/raphaelsc/scylla.git abort_reshape_v1

Commit Bot

unread,
Aug 2, 2021, 3:00:16 PMAug 2
to scylla...@googlegroups.com, Raphael S. Carvalho
From: Raphael S. Carvalho <raph...@scylladb.com>
Committer: Raphael S. Carvalho <raph...@scylladb.com>
Branch: next

api: make compaction manager api available earlier

That will be needed for aborting reshape on boot.

Refs #7738.

Signed-off-by: Raphael S. Carvalho <raph...@scylladb.com>

---
diff --git a/api/api.cc b/api/api.cc
--- a/api/api.cc
+++ b/api/api.cc
@@ -182,13 +182,20 @@ future<> set_server_gossip_settle(http_context& ctx) {
});
}

-future<> set_server_done(http_context& ctx) {
+future<> set_server_compaction_manager(http_context& ctx) {
auto rb = std::make_shared < api_registry_builder > (ctx.api_doc);

return ctx.http_server.set_routes([rb, &ctx](routes& r) {
rb->register_function(r, "compaction_manager",
"The Compaction manager API");
set_compaction_manager(ctx, r);
+ });
+}
+
+future<> set_server_done(http_context& ctx) {
+ auto rb = std::make_shared < api_registry_builder > (ctx.api_doc);
+
+ return ctx.http_server.set_routes([rb, &ctx](routes& r) {
rb->register_function(r, "lsa", "Log-structured allocator API");
set_lsa(ctx, r);

diff --git a/api/api_init.hh b/api/api_init.hh
--- a/api/api_init.hh
+++ b/api/api_init.hh
@@ -87,6 +87,7 @@ future<> set_server_storage_proxy(http_context& ctx, sharded<service::storage_se
future<> set_server_stream_manager(http_context& ctx);
future<> set_server_gossip_settle(http_context& ctx);
future<> set_server_cache(http_context& ctx);
+future<> set_server_compaction_manager(http_context& ctx);
future<> set_server_done(http_context& ctx);

}
diff --git a/main.cc b/main.cc
--- a/main.cc
+++ b/main.cc
@@ -1031,6 +1031,9 @@ int main(int ac, char** av) {

Commit Bot

unread,
Aug 2, 2021, 3:00:17 PMAug 2
to scylla...@googlegroups.com, Raphael S. Carvalho
From: Raphael S. Carvalho <raph...@scylladb.com>
Committer: Raphael S. Carvalho <raph...@scylladb.com>
Branch: next

compaction: Allow reshape to be aborted

Now reshape can be aborted on either boot or refresh.

The workflow is:
1) reshape starts
2) user notices it's taking too long
3) nodetool stop RESHAPE

the good thing is that completed reshape work isn't lost, allowing
table to enjoy the benefits of all reshaping done up to the abortion
point.

Fixes #7738.

Signed-off-by: Raphael S. Carvalho <raph...@scylladb.com>

---
diff --git a/compaction/compaction_manager.cc b/compaction/compaction_manager.cc
--- a/compaction/compaction_manager.cc
+++ b/compaction/compaction_manager.cc
@@ -317,6 +317,7 @@ future<> compaction_manager::run_custom_job(column_family* cf, sstables::compact
f.get();
} catch (sstables::compaction_stop_exception& e) {
cmlog.info("{} was abruptly stopped, reason: {}", task->type, e.what());
+ throw;
} catch (...) {
cmlog.error("{} failed: {}", task->type, std::current_exception());
throw;
diff --git a/sstables/sstable_directory.cc b/sstables/sstable_directory.cc

Commit Bot

unread,
Aug 2, 2021, 8:26:30 PMAug 2
to scylla...@googlegroups.com, Raphael S. Carvalho
From: Raphael S. Carvalho <raph...@scylladb.com>
Committer: Raphael S. Carvalho <raph...@scylladb.com>
Branch: master

Commit Bot

unread,
Aug 2, 2021, 8:26:30 PMAug 2
to scylla...@googlegroups.com, Raphael S. Carvalho
From: Raphael S. Carvalho <raph...@scylladb.com>
Committer: Raphael S. Carvalho <raph...@scylladb.com>
Branch: master

api: make compaction manager api available earlier

That will be needed for aborting reshape on boot.

Refs #7738.

Signed-off-by: Raphael S. Carvalho <raph...@scylladb.com>

---

Benny Halevy

unread,
Aug 8, 2021, 2:09:07 AMAug 8
to Raphael S. Carvalho, Avi Kivity, scylladb-dev
Propagating the exception caused a regression
when scylla shuts down distributed_loader::populate_column_family
(https://github.com/scylladb/scylla/issues/9158)

We need to handle the propagated exception
for clean shutdown.
Reply all
Reply to author
Forward
0 new messages