Add mnesia:repair_continuation for continuing a select in a new activity

[dszoboszlay]

I think the problem this PR fixes is best explained via the use case behind it. I have a web app implementing paginated search results using mnesia:select/4:

{Rows1, C1} = mnesia:activity(async_dirty, fun mnesia:select/4, [Tab, MatchSpec, NObjects, read]),
draw_table(Rows1),

%% and when the user clicks next:
{Rows2, C2} = mnesia:activity(async_dirty, fun mnesia:select/1, [C1]),
draw_table(Rows2),

%% and so on

Unfortunately this doesn't work when the mnesia:select/1 call is done from a different process (which is typical for web apps). It crashes with {aborted,wrong_transaction}, because the activity id for async_dirty contains the pid, so it will only match in the same process.

I understand this check is important for transactional contexts: reusing a continuation in a different transaction would break the transactional guarantees. However, for dirty (or ets) contexts it simply doesn't make sense.

One option would be to remove the check on the activity context in non-transactional uses. But that seems to be a bit too much magic to me, and also opens up a lot of questions about edge cases like continuing a select that was started in a transactional context in a non-transactional activity.

I decided on adding an explicit call, mnesia:repair_continuation/1 (named after ets:repair_continuation/1), for importing a continuation into the current activity. It works across both transactional and non-transactional contexts. Maybe the first use case would not make much sense in practice, but if you actually need it, well, feel free to do it at your own risk! And since the new feature is introduced in a new function, it will not break existing code either.

So this PR makes possible to do things like this, regardless which process uses C2:

{Rows1, C1} = mnesia:activity(async_dirty, fun mnesia:select/4, [Tab, MatchSpec, NObjects, read]),
draw_table(Rows1),

%% and when the user clicks next:
{Rows2, C2} = mnesia:activity(async_dirty, fun () -> mnesia:select(mnesia:repair_continuation(C1)) end),
draw_table(Rows2),

%% and so on

I tried to describe mnesia:repair_continuation/1 in the docs as well as I could, but suggestions for improvements are welcome.

[OTP-Maintainer]

The summary line of the commit message is too long and/or ends with a "."
Make sure the whole message follows the guidelines here: https://github.com/erlang/otp/wiki/Writing-good-commit-messages.

Bad message: Add mnesia:repair_continuation/[12] for continuing a select in a new activity

---

I am a script, I am not human

---

[dszoboszlay]

Fixed commit message format.

[OTP-Maintainer]

Patch has passed first testings and has been assigned to be reviewed

---

I am a script, I am not human

---

[dgud]

I'm not sure I like that you allow the function to be called in transactions, as you said the use case
does not make a lot of sense.

I can see the use case for dirty selection so I'm not opposed of a fix to that part.
If we don't allow the transaction use case should we still add a new function or just automagicly fix the continuation?

[dszoboszlay]

First of all, I don't like automagically fixing things, because

• it is magic
• it breaks backward compatibility: activities that used to abort before would start working
• you need to define the scope of the magic: is it for example OK to continue a transactional select in a dirty context? Or what about continuing an initially dirty select in a transactional context?

On the other hand, limiting the repair to dirty contexts only would let us integrate the repair into mnesia:select_cont/3 like this:

select_cont(Tid,Ts,State=#mnesia_select{}) when ?some_guard_for_dirty_contexts(Tid) ->
    RepairedState = State#mnesia_select{tid = Tid, written = [], spec = undefined, type = undefined},
    select_cont(Tid,Ts,RepairedState);

This would work with mnesia_frag too.

[dgud]

Ok so remove the transaction support.

I'll let you make a decision about the auto-repair or new-function,
you have thought more about it and I don't really have an opinion.

I think backwards compatibility argument can be ignored, I can't see that a dirty select continuation call suddenly starts working is a big problem.

[dszoboszlay]

OK, but what do you mean by "remove transaction support"?

A continuation can be created in one of 5 possible contexts:

• transaction
• sync_transaction
• dirty
• async_dirty
• ets

And it may be used the next time in one of these 5 contexts as well. So we have 25 combinations, which ones would you like to deny?

[dgud]

ContinueStates should not be allowed to be repaired and continued in a
new transaction or a new sync_transaction context,
i.e. activity's where mnesia:is_transaction() returns true.

ContinueStates created in transactions and repaired in *dirty|ets activity,
could be disallowed as well sounds like a strange use case for me.

On Wed, Oct 5, 2016 at 4:34 PM Dániel Szoboszlay notifications@github.com
wrote:

OK, but what do you mean by "remove transaction support"?

A continuation can be created in one of 5 possible contexts:

• transaction
• sync_transaction
• dirty
• async_dirty
• ets

And it may be used the next time in one of these 5 contexts as well. So we
have 25 combinations, which ones would you like to deny?

—
You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub
#1184 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAE8KQ9B-v6r6c_hNJMki8UHJgj4Nu5xks5qw7WQgaJpZM4KLKxT
.

[mikpe]

As the person pushing for us (Klarna) to use this feature (and we're doing a field test with a version of it right now), the only use case I care about is async_dirty-to-async_dirty. That is, a continuation created by a select in one async_dirty context shall work with a select/1 in another async_dirty context. Crossing between dirty and non-dirty context is not needed right now, and I frankly have a hard time imagining what the corresponding use case might be. However, if supporting that can be done safely and without complicating the code I'm not opposed to doing so.

[dszoboszlay]

Now the import happens automatically in dirty contexts regardless of the originating context of the continuation.

[OTP-Maintainer]

Patch has passed first testings and has been assigned to be reviewed

---

I am a script, I am not human

---