#47602 Make ldbm_back_seq independently support transactions
Closed: wontfix None Opened 10 years ago by mreynolds.

This is part two of ticket 47598.

ldbm_back_seq should be able to start a transaction, handle transaction deadlocks/retries, and commit/abort.


Description: If ldbm_back_seq is called as a child of transaction, it
fails to access the on-going transaction data. This patch picks up
the parent transaction if any, and it calls dblayer_read_txn_begin with
the parent transaction. If the read transaction is aborted by DEADLOCK,
it retries.

Reviewed by Mark (Thank you!!)

Pushed to master:
dde9abe..a037d1c master -> master
commit a037d1c

The commit is being done too early, this can cause a dead lock in the changelog db. Working on a new patch...

Thanks for fixing the bug, Mark!

I have one concern...
Now, dblayer_read_txn_commit is called with this condition (line 323).
{{{
322 / if success finally commit the transaction /
323 if (return_value == 0 && txn.back_txn_txn) {
324 dblayer_read_txn_commit(be, &txn);
325 }
}}}
The abort is called with this condition.
{{{
238 238 if ((0 == return_value) || (DB_NOTFOUND == return_value)) {
...
267 } else {
268 if (txn.back_txn_txn) {
269 dblayer_read_txn_abort(be, &txn);
270 }
}}}
When return_value is DB_NOTFOUND, both dblayer_read_txn_commit and dblayer_read_txn_abort are not called and the transaction might be leaked?

Replying to [comment:10 nhosoi]:

Thanks for fixing the bug, Mark!

I have one concern...
Now, dblayer_read_txn_commit is called with this condition (line 323).
{{{
322 / if success finally commit the transaction /
323 if (return_value == 0 && txn.back_txn_txn) {
324 dblayer_read_txn_commit(be, &txn);
325 }
}}}
The abort is called with this condition.
{{{
238 238 if ((0 == return_value) || (DB_NOTFOUND == return_value)) {
...
267 } else {
268 if (txn.back_txn_txn) {
269 dblayer_read_txn_abort(be, &txn);
270 }
}}}
When return_value is DB_NOTFOUND, both dblayer_read_txn_commit and dblayer_read_txn_abort are not called and the transaction might be leaked?

You are correct, new patch attached!

git merge ticket47602
Updating 6a1b10d..958b4ed
Fast-forward
ldap/servers/slapd/back-ldbm/seq.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

git push origin master
6a1b10d..958b4ed master -> master

commit 958b4ed
Author: Mark Reynolds mreynolds@redhat.com
Date: Fri Jun 13 13:48:50 2014 -0400

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.3.3 - 1/14 (January)

7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/939

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

3 years ago

Login to comment on this ticket.

Metadata