Duplicated save-point names in parallel sub-transactions

[Tol1]

I have 3 helper functions, A, B and C, and all of them start transaction, for they can all be called individually. A calls C, so when A is called, C starts savepoint with name level_1.
Then I have one case, where I call both A and B in same time, using function D. I want everyting to be run in transaction, so D starts transaction, and calls A and B using Promise.all.
Now comes the problem: Both A and B start a savepoint with name level_1, and C starts a savepoint with name level_2. Per postgresql spec, if duplicate savepoint names are used, RELEASE SAVEPOINT releases last defined savepoint first. Now I can end in situation, where finishing B releases savepoint defined in A, and thus releases also sub-savepoint defined in C, giving me error no such savepoint when C actually finishes.
Everything works when calling A and C sequentically, but this made us scratch our heads when searching for unhandled promises.

Expected behavior

Every savepoint gets an unique name

Actual behavior

Savepoints get unique name only on current branch

Environment

• Version of pg-promise: 10.8.1
• OS type (Linux/Windows/Mac): Linux
• Version of Node.js: 14.15.1

[xscode-auto-reply]

Thanks for opening a new issue. The team has been notified and will review it as soon as possible.
For urgent issues and priority support, visit https://xscode.com/vitaly-tomilov/pg-promise

[vitaly-t]

You need to chain your helper functions correctly, supporting optional transaction context, then it will work as expected.

If you include simplified version of those here, then I can show what's wrong with them.

[Tol1]

I know I can use txIf to gain optional transaction, and I know about nested transaction limitations. But your documentation says library handles savepoints, "with unique savepoint names". Those are not unique, those are only unique on current path.
I suggest you should indicate more clearly on documentation that the savepoint names are not unique, but only tells the level of transaction, or then use truly unique names.

[vitaly-t]

As I explained earlier, most likely you are not chaining them correctly. I cannot be more accurate, since you are not providing the troubling code example. Include the code, then we should make progress here.

I suggest you should indicate more clearly on documentation that the savepoint names are not unique, but only tells the level of transaction, or then use truly unique names

That would imply that invalid transaction chaining is ok, but it is not. Let's not jump ahead of ourselves, and look at the actual code first, which you should provide.

[vitaly-t]

@Tol1 Are you still planning to add a code example? If not, then we should close it, because we cannot make progress without one.

[vitaly-t]

Closing, because no functioning example provided.

[vitaly-t]

Since the author failed to provide any code example, I finally got time to try and produce the issue myself...

I can confirm that now that if you try to execute multiple sub-transactions in parallel, the save-points end up with overlapping names, which results in invalid release of save-points.

I suggest you should indicate more clearly on documentation that the savepoint names are not unique

It's not that documentation isn't clear enough, it didn't account for that special case, neither did the tests.

So you can consider that a bug or limitation, though the use case is quite unique. Also, it has a simple work-around, by using the async-await syntax on the same-level transactions, because allowing promises run in parallel in this case accomplishes nothing - you still execute against the same open connection, which means the IO is synchronized for the queries, so no performance loss there.

i.e. you take the following problematic code:

db.tx(t => {
    const a = t.tx(t1 => { });
    const b = t.tx(t1 => { }); 
    const c = t.tx(t1 => { });

    return t.batch([a, b, c]);
})

=>

savepoint level_1
savepoint level_1
savepoint level_1

release savepoint level_1
release savepoint level_1
release savepoint level_1

and replace it with this:

db.tx(async t => {
    const a = await t.tx(t1 => { });
    const b = await t.tx(t1 => { });
    const c = await t.tx(t1 => { });

    return {a, b, c};
})

=>

savepoint level_1
release savepoint level_1

savepoint level_1
release savepoint level_1

savepoint level_1
release savepoint level_1

and then no more problem.

---

I may later at some point look into improving the library, to make save points fully unique.

[vitaly-t]

After a bit more investigation, I'm going to close this again, because trying to parallel sub-transactions via promise.all or something like that will never work right.

Consider the following SQL:

begin;
savepoint one;
savepoint two;
release savepoint one;
release savepoint two;
commit;

It will throw error on line release savepoint two, because savepoint two seizes to exist, once release savepoint one is executed, because it was created inside that savepoint.

This is what will keep happening, if you try parallel savepoints inside a transaction, you will always lose control of savepoints nesting logic.

Therefore, you should never do that to begin with, and just sync sub-transactions, using async-await syntax, as shown above.

[vitaly-t]

In version 10.9.3, I changed how savepoints are named - see Transaction Limitations. Savepoints now use format sp_x_y, as explained there.