Basic multi modes for MULTI/EXEC #796

dranikpg · 2023-02-14T09:52:42Z

This commit adds the notion of multi transaction modes that allow controlling the execution and
locking behaviour of multi transactions.
In general, there are four modes:

GLOBAL: all commands run within a global transaction. There is no need for recording locks. Lua scripts can theoretically run with undeclared keys.
LOCK_AHEAD: the transaction locks all keys ahead likewise to a regular transaction and schedules itself.
LOCK_INCREMENTAL: the transaction determines what shards it has keys in and schedules itself on those shards, but locks only when accessing a new key. This allows other transactions to run ooo alonside with a big multi-transaction that accesses a contended key only at its very end.
NON_ATOMIC: all commands run separately, no atomicity is provided, likewise to a pipeline

This commit only adds support for the first 3 modes to EXEC commands.

Signed-off-by: Vladislav Oleshko [email protected]

dranikpg · 2023-02-14T20:59:19Z

I'll stop growing the PR on this state, currently it has

A MultiMode defined
A new ACTIVE flag for shard data
Basic StartMulti wrappers that are used in all multi cases
Support for 3 modes of execution for MULTI...EXEC

So far it seems to work with a single instance. The bad news is that currently replication tests pass only with the LOCK_INCR_GLOBAL mode (which was the default mode before)

There is still some unresolved stuff and details, but you can take a look at it on a high level

src/server/db_slice.cc

dranikpg · 2023-02-14T21:02:14Z

src/server/dragonfly_test.cc

+  // TODO: We have no way of measuring now.
+  // EXPECT_EQ(2, GetDebugInfo().shards_count);


The issue is:

this info was set after InitByArgs inside dispatch

We deduce the number of active shards only in the handler

So we probably need to update the debug info inside the handler itself?

src/server/dragonfly_test.cc

dranikpg · 2023-02-14T21:05:42Z

src/server/main_service.cc

@@ -1448,7 +1509,7 @@ void Service::RegisterCommands() {
             &EvalValidator)
      << CI{"EVALSHA", CO::NOSCRIPT | CO::VARIADIC_KEYS, -3, 3, 3, 1}.MFUNC(EvalSha).SetValidator(
             &EvalValidator)
-      << CI{"EXEC", kExecMask, 1, 0, 0, 0}.MFUNC(Exec)
+      << CI{"EXEC", CO::LOADING | CO::NOSCRIPT, 1, 0, 0, 1}.MFUNC(Exec)


First, this is no longer GLOBAL by default

Second, the step is now 1. This is because some parts of tx rely on it for reading arguments. We virtually "append" arguments to EXEC to schedule it ahead for example

where we do that?

if we lock ahead

We read the keys from shard args based on their 'step'. If we schedule ahead with EXEC, then we store the keys inside the per-shard args

I would like to add a comment about it here or where you do it

src/server/transaction.cc

dranikpg · 2023-02-14T21:10:29Z

src/server/transaction.h

  struct PerShardCache {
+    bool requested_active = false;


This is what I added to schedule on shards without having the keys.

If this flag is set, the new ACTIVE flag is set as well even if there are no arguments.

TODO: need to add a comment about this

src/server/main_service.cc

romange

looks good. I have not checked the correctness - it's almost impossible but I guess the tests are passing :)

src/server/main_service.cc

romange · 2023-02-16T15:42:23Z

src/server/main_service.cc

@@ -1448,7 +1509,7 @@ void Service::RegisterCommands() {
             &EvalValidator)
      << CI{"EVALSHA", CO::NOSCRIPT | CO::VARIADIC_KEYS, -3, 3, 3, 1}.MFUNC(EvalSha).SetValidator(
             &EvalValidator)
-      << CI{"EXEC", kExecMask, 1, 0, 0, 0}.MFUNC(Exec)
+      << CI{"EXEC", CO::LOADING | CO::NOSCRIPT, 1, 0, 0, 1}.MFUNC(Exec)


where we do that?

src/server/transaction.cc

romange · 2023-02-17T07:30:39Z

I would like to squash these commits and then have a good commit message giving a summary of this change.
Could be reused with changes in transaction.md.

What are the modes that you add.
That you make global property deduced at runtime
Give context - i.e. "lock ahead is relevant for multi-exec transactions and it reduces their locking coverage from global to per key locks during scheduling step"
Explain that incremental is similar to lock-ahead but delays the locking to when each command within the multi-exec transaction is executed.
which modes are applicable to lua transactions.

The motivation - for us to look at the commit a month from now and understand the logic.

This commit adds the notion of multi transaction modes that allow controlling the execution and locking behaviour of multi transactions. In general, there are four modes: - GLOBAL: all commands run within a global transaction. There is no need for recording locks. Lua scripts can theoretically run with undeclared keys. - LOCK_AHEAD: the transaction locks all keys ahead likewise to a regular transaction and schedules itself. - LOCK_INCREMENTAL: the transaction determines what shards it has keys in and schedules itself on those shards, but locks only when accessing a new key. This allows other transactions to run ooo alonside with a big multi-transaction that accesses a contended key only at its very end. - NON_ATOMIC: all commands run separately, no atomicity is provided, likewise to a pipeline This commit only adds support for the first 3 modes to EXEC commands. Signed-off-by: Vladislav Oleshko <[email protected]>

dranikpg force-pushed the update-multi branch from df0c63d to 25045f4 Compare February 14, 2023 20:56

dranikpg commented Feb 14, 2023

View reviewed changes

dranikpg requested a review from romange February 14, 2023 21:10

dranikpg changed the title ~~Multi run modes [WIP]~~ Multi run modes [WIP 1/2] Feb 14, 2023

dranikpg commented Feb 15, 2023

View reviewed changes

src/server/main_service.cc Show resolved Hide resolved

dranikpg force-pushed the update-multi branch 2 times, most recently from 19a6958 to 1742104 Compare February 16, 2023 14:33

dranikpg marked this pull request as ready for review February 16, 2023 14:33

dranikpg changed the title ~~Multi run modes [WIP 1/2]~~ Basic multi modes for MULTI/EXEC Feb 16, 2023

romange reviewed Feb 16, 2023

View reviewed changes

dranikpg requested a review from romange February 16, 2023 19:46

dranikpg force-pushed the update-multi branch from 0923819 to 909ed05 Compare February 17, 2023 15:27

dranikpg force-pushed the update-multi branch from 909ed05 to d63fd68 Compare February 18, 2023 10:19

romange approved these changes Feb 18, 2023

View reviewed changes

dranikpg merged commit 4ef06e7 into dragonflydb:main Feb 18, 2023

dranikpg deleted the update-multi branch February 27, 2023 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic multi modes for MULTI/EXEC #796

Basic multi modes for MULTI/EXEC #796

dranikpg commented Feb 14, 2023 •

edited by romange

Loading

dranikpg commented Feb 14, 2023

dranikpg Feb 14, 2023

dranikpg Feb 14, 2023

romange Feb 16, 2023

dranikpg Feb 16, 2023

romange Feb 17, 2023

dranikpg Feb 14, 2023

romange left a comment

romange Feb 16, 2023

romange commented Feb 17, 2023

		// TODO: We have no way of measuring now.
		// EXPECT_EQ(2, GetDebugInfo().shards_count);

Basic multi modes for MULTI/EXEC #796

Basic multi modes for MULTI/EXEC #796

Conversation

dranikpg commented Feb 14, 2023 • edited by romange Loading

dranikpg commented Feb 14, 2023

dranikpg Feb 14, 2023

Choose a reason for hiding this comment

dranikpg Feb 14, 2023

Choose a reason for hiding this comment

romange Feb 16, 2023

Choose a reason for hiding this comment

dranikpg Feb 16, 2023

Choose a reason for hiding this comment

romange Feb 17, 2023

Choose a reason for hiding this comment

dranikpg Feb 14, 2023

Choose a reason for hiding this comment

romange left a comment

Choose a reason for hiding this comment

romange Feb 16, 2023

Choose a reason for hiding this comment

romange commented Feb 17, 2023

dranikpg commented Feb 14, 2023 •

edited by romange

Loading