Ansible restore script seems to be broken

[Michal-Leszczynski]

Conversation under issue showed, that Ansible restore script lacks some steps in order to ensure restore success.
In terms of restoring schema, there are two options:

If there was no schema disagreement at the time of taking the backup then we're fine, no need for repair or gc grace seconds tricks on schema tables. Unlike user tables, schema tables automatically repair themselves across nodes.

This one is connected to #3323

If backup was taken during schema change and there is disagreement between nodes, we may run into the problems mentioned earlier. You could avoid this by giving each node sstables from all the nodes when restoring schema, simulating repair.

This one would require some changes in SSTables upload procedure.

In terms of restoring table contents, SM should:

• disable gc_grace_seconds or set tombstone_gc = {'mode':'disabled'} via alter query
• load SSTables
• repair
• enable gc_grace_seconds or set tombstone_gc = {'mode':'timeout'}

Without those changes, it seems that data resurrection might occur.

[vladzcloudius]

I would be careful about changing the "restore script" (a.k.a. restore.yml Ansible playbook).

Changing the gc_grace_seconds is the only thing that makes sense to put into a generic flow.

Having to run a repair every time one restores from a snapshot kills the purpose of "cluster cloning" procedure.

"schema disagreement during the snapshot taking" in many cases can be avoided by the user and hence all heavy additions to the procedure mentioned above that are only relevant to this specific corner case may be safely skipped.

Please, when you implement the changes make sure to properly document all that and allow skipping heavy extra steps in case schema state during the backup is guaranteed to be in sync.

cc @karol-kokoszka @tzach @tgrabiec @asias

[tgrabiec]

Running repair is also needed to restore user tables.

[vladzcloudius]

Running repair is also needed to restore user tables.

No, it's not.

[tgrabiec]

No, it's not.

Why not?

[vladzcloudius]

No, it's not.

Why not?

Because the way we restore guarantees that the consistency of the data is exactly the same as it was when the snapshot has been created.

[tgrabiec]

What about the scenario described here: scylladb/scylladb#13041 (comment)

Restoring a snapshot as-is means you violated the rule which requires you to run repair every once in gc_grace_seconds, if you restored a snapshot which was taken earlier than gc_grace_seconds ago.

[vladzcloudius]

What about the scenario described here: scylladb/scylladb#13041 (comment)

Restoring a snapshot as-is means you violated the rule which requires you to run repair every once in gc_grace_seconds, if you restored a snapshot which was taken earlier than gc_grace_seconds ago.

I think I've already referred to this here: #3324 (comment)

But let me clarify it even more:

Increasing the gc_grace_seconds value is enough and even this has to be optional.
Running a repair is supposed to be outside of the scope of the restoration playbook.
It's up to a user to decide if it needs to be run (e.g. if there are no tombstones in the snapshot at all) and, if "yes", when it needs to be executed.

[tgrabiec]

Increasing the gc_grace_seconds value is enough and even this has to be optional.

What do you mean by that exactly? If we're talking about changing it to +inf, then to avoid running repair you have to leave it at that (bad). If you restore it after loading the snapshot without running repair, you're exposed to tombstone compaction and data resurrection.

[tgrabiec]

Having to run a repair every time one restores from a snapshot kills the purpose of "cluster cloning" procedure.

Why? Is cloning the cluster done in order to avoid running repair?

[vladzcloudius]

Increasing the gc_grace_seconds value is enough and even this has to be optional.

What do you mean by that exactly? If we're talking about changing it to +inf, then to avoid running repair you have to leave it at that (bad). If you restore it after loading the snapshot without running repair, you're exposed to tombstone compaction and data resurrection.

I meant to leave it at +inf (unless user explicitly asked not to change it) and allow the user to decide what to do about that - including running a repair.

[vladzcloudius]

Having to run a repair every time one restores from a snapshot kills the purpose of "cluster cloning" procedure.

Why? Is cloning the cluster done in order to avoid running repair?

Cloning the cluster is meant to be fast.
Running a repair is a slow and heavy procedure.

[tomer-sandler]

I spoke with @tzach last week and this is something that is actively being worked on. Since it's an "existing" condition and not a regression, Engineering will most likely not delay SM 3.1 for this fix, so it may only come at SM version 3.1.x (where x > 0).
The solution will either that SM will run repair at the end of the restore from backup (using load&stream --> SM 3.1.x), or it will be documented as part of the restore procedure that a repair must be executed by the user.