https://bugs.openldap.org/show_bug.cgi?id=10395

kero <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1130|0                           |1
        is obsolete|                            |

--- Comment #15 from kero <[email protected]> ---
Created attachment 1131
  --> https://bugs.openldap.org/attachment.cgi?id=1131&action=edit
Allow multiple nested read transactions from a write transaction review from
comment #14

Hello Howard,

> LOCK_MUTEX is used for the process-shared mutexes, which may not always be 
> pthread mutexes. You should just use pthread_mutex_lock here since you don't 
> need an interprocess mutex.

Thanks for the info. I am already using pthread_mutex_lock. So, that's fine.

> You can free a NULL pointer, that's standard C idiom. No need to 
> conditionalize that.

Thanks for reminding me. However, I made sure not to allocate dirty_list and
mt_free_pgs, and to use the parent's ones. I made sure not to free parents'
lists when the transaction is a nested read-only one.

> It occurs to me that the reader table is irrelevant in this case. The reader 
> table's purpose is to prevent a write txn from reusing pages that read txns 
> may still need. But since in this case, the write txn is the parent and it 
> cannot do anything while it has child read txns, there's nothing to worry 
> about there. Which means these read txns can be used freely, independent of 
> the readers table and any maxreaders setting.
>
> But it could get confusing if these read txns are kept around (using 
> txn_reset/txn_renew) and used outside of a parent write txn, because then 
> they would need their slot in the reader table (assuming a default 
> environment, without NOTLS).

Thanks for the deeper explanation of the problem. I now better understand. I
checked, and it seems that maxreaders is already ignored by nested read
transactions, as it is checked in mdb_txn_renew0, and only no-parent (+RDONLY)
txns enter it. However, I see that we use calloc to allocate any transaction
with a parent, which means that mt_u.reader is set to NULL for our nested read
transaction. Am I right that everything is working fine, as the parent
transaction is locked while child transactions are alive?

> As above, it's probably fine to use nested read txns regardless. But unless 
> specially handled otherwise, readtxns created as a child txn probably can't 
> be reused later as independent readtxns.

I updated the mdb_txn_reset function to ignore nested read-only transactions
and updated the documentation accordingly. I updated the documentation of
mdb_txn_renew, as it throws an EINVAL because mdb_txn_reset skips nested read
transactions.

> +   if (!txn->mt_parent && F_ISSET(txn->mt_flags, MDB_TXN_RDONLY)) {

Should I do anything with this? Is it correct, or should I handle that
differently?

> Also, the documentation for mdb_txn_begin in lmdb.h should be updated.

Done. I updated the documentation of mdb_txn_begin, mdb_txn_reset, and
mdb_txn_renew accordingly.

Thank you and have a nice day,
kero

-- 
You are receiving this mail because:
You are on the CC list for the issue.

Reply via email to