internal_keyspace extention: enhance the semantics also to flushes (dirty_memory_manager) #14529
Comments
|
/cc @elcallio |
commit 7c8c020 introduced a new type of a keyspace, an internal keyspace It defined the semantics for this internal keyspace, this keyspace is somewhat a hybrid between system and user keyspace. Here we extend the semantics to include also flushes, meaning that flushes will be done using the system dirty_mamory_manager. This is in order to allow inter dependencies between internal tables and user tables and prevent deadlocks. One example of such a deadlock is our `replicated_key_provider` encryption on the enterprise version. The deadlock occur because in some circumstances, an encrypted user table flush is dependant upon the `encrypted_keys` table being flushed but since the requests are serialized, we get a deadlock. Tests: unit tests dev + debug The deadlock dtest reproducer: encryption_at_rest_test.py::TestEncryptionAtRest::test_reboot Fixes scylladb#14529 Signed-off-by: Eliran Sinvani <eliransin@scylladb.com>
commit 7c8c020 introduced a new type of a keyspace, an internal keyspace It defined the semantics for this internal keyspace, this keyspace is somewhat a hybrid between system and user keyspace. Here we extend the semantics to include also flushes, meaning that flushes will be done using the system dirty_mamory_manager. This is in order to allow inter dependencies between internal tables and user tables and prevent deadlocks. One example of such a deadlock is our `replicated_key_provider` encryption on the enterprise version. The deadlock occur because in some circumstances, an encrypted user table flush is dependant upon the `encrypted_keys` table being flushed but since the requests are serialized, we get a deadlock. Tests: unit tests dev + debug The deadlock dtest reproducer: encryption_at_rest_test.py::TestEncryptionAtRest::test_reboot Fixes scylladb#14529 Signed-off-by: Eliran Sinvani <eliransin@scylladb.com> Closes scylladb#14547
commit 7c8c020 introduced a new type of a keyspace, an internal keyspace It defined the semantics for this internal keyspace, this keyspace is somewhat a hybrid between system and user keyspace. Here we extend the semantics to include also flushes, meaning that flushes will be done using the system dirty_mamory_manager. This is in order to allow inter dependencies between internal tables and user tables and prevent deadlocks. One example of such a deadlock is our `replicated_key_provider` encryption on the enterprise version. The deadlock occur because in some circumstances, an encrypted user table flush is dependant upon the `encrypted_keys` table being flushed but since the requests are serialized, we get a deadlock. Tests: unit tests dev + debug The deadlock dtest reproducer: encryption_at_rest_test.py::TestEncryptionAtRest::test_reboot Fixes scylladb#14529 Signed-off-by: Eliran Sinvani <eliransin@scylladb.com> Closes scylladb#14547
|
Backport to 2023.1 queued as 0954c5d18d998b90213589d32b35f54f5e81a2a9. |
Dequeued, breaks the build. |
@eliransin - who's looking at this? |
I will look at this, missed the fact it breaks 2023.1 |
|
Where is the broken build? |
|
This is in 2023.1 (I am not sure about the dequing...) Also - we have https://github.com/scylladb/scylla-enterprise/issues/3334 to manage 2023.1 backport of this patch. |
|
Anything related to enterprise should be discussed on its own tracker. |
|
Should this be backported, if so, why? Issues like "X should be Y" are unhelpful, it's impossible to say if they are fixes or enhancements. |
|
Not further than to 2023, in which it is already. |
Commit 7c8c020 introduced a new type of a keyspace, an internal keyspace:
It is said in the commit message:
It should be extended also to:
4) should not compete with user keyspace for flushes
This is to prevent deadlocks,
The explanation, this extension was created in order to allow for "soft system_tables", which means that their schema as well
as the data is not local. The semantics didn't concern at all if flushes should compete with user tables of not.
pure system tables can be assumed not to compete with user table flushes thus if a specific user table flush would like to
force a system table flush as a condition to it's own flush it can do so without the fear of a deadlock that will be resulted if
the flush of the system table is serialized after the user table flush.
We actually do this in enterprise in tables that are encrypted using
replicated_key_providerand we indeed deadlock sinceinternal keyspaces does compete with user table flushes.
Since they are considered to be a "high priority keyspaces" anyway we should extend the semantics as aforementioned above.
I am also labeling it as a bug since this missing additional semantic causes us a real deadlock in
replicated_key_provide.The text was updated successfully, but these errors were encountered: