#303 make DNA range requests work with transactions
Closed: wontfix None Opened 12 years ago by rmeggins.

DNA range requests need to work with transactions when the DNA plugin is made into a betxn plugin.


The real issue is that we need to avoid doing a DNA range request operation from within a transaction. We do not want to be performing network operations while we are within the transaction, as this can lock out changes to the database longer than is necessary.

The goal should be to do range requests at the preop stage and actual DNA value allocation at the betxnpreop stage. The plug-in is currently written like this, but I'm not sure if there is a possibility that we can still do a range request from within a transaction.

Fix Description:
1. pre_op: Adding missing dnatypes (e.g., uidNumber) should be done
in the pre op phase (outside of the transaction) to satisfy the
schema checking. To avoid calling the internal search for modify,
set the target entry before calling pre op plugin in op_shared_
modify (modify.c).
Also, if the operation is a replication op, the pre_op is skipped.
2. post_op: Moving dna_config_check_post_op to BE_TXN_POST_OP.
If it is an internal operation, the dna post op is being skipped
to avoid self re-entrant deadlock.
3. Fixed memory leaks on DNA_NEEDS_UPDATE and an uninitialized
variable access.

Replying to [comment:3 nhosoi]:

Fix Description:
1. pre_op: Adding missing dnatypes (e.g., uidNumber) should be done
in the pre op phase (outside of the transaction) to satisfy the
schema checking. To avoid calling the internal search for modify,
set the target entry before calling pre op plugin in op_shared_
modify (modify.c).

Instead of this, could you use a bepreop instead of a regular preop? Then we wouldn't need to set the preop entry in modify.c

Also, if the operation is a replication op, the pre_op is skipped.
2. post_op: Moving dna_config_check_post_op to BE_TXN_POST_OP.
If it is an internal operation, the dna post op is being skipped
to avoid self re-entrant deadlock.
3. Fixed memory leaks on DNA_NEEDS_UPDATE and an uninitialized
variable access.

Replying to [comment:4 rmeggins]:

Replying to [comment:3 nhosoi]:

Fix Description:
1. pre_op: Adding missing dnatypes (e.g., uidNumber) should be done
in the pre op phase (outside of the transaction) to satisfy the
schema checking. To avoid calling the internal search for modify,
set the target entry before calling pre op plugin in op_shared_
modify (modify.c).

Instead of this, could you use a bepreop instead of a regular preop?

Unfortunately, we cannot. DNA allows to user to add or modify an entry without uidNumber or gidNumber. The pre-op fills the missing attributes and the entry is passed to the schema check. If we move the current DNA pre op to bepreop, the add/modify operation fails due to the schema violation...

Replying to [comment:5 nhosoi]:

Replying to [comment:4 rmeggins]:

Replying to [comment:3 nhosoi]:

Fix Description:
1. pre_op: Adding missing dnatypes (e.g., uidNumber) should be done
in the pre op phase (outside of the transaction) to satisfy the
schema checking. To avoid calling the internal search for modify,
set the target entry before calling pre op plugin in op_shared_
modify (modify.c).

Instead of this, could you use a bepreop instead of a regular preop?

Unfortunately, we cannot. DNA allows to user to add or modify an entry without uidNumber or gidNumber. The pre-op fills the missing attributes and the entry is passed to the schema check. If we move the current DNA pre op to bepreop, the add/modify operation fails due to the schema violation...

Where is this done? In both the add and the modify case, the only place where slapi_entry_schema_check() is called is after the BE_PRE operation functions have been called.

Thanks for your patience, Rich! I misunderstood your comments. I thought you were suggesting to move pre_op to betxn_pre_op, which I already tried and failed... :p Now, the pre_op_add/modify are moved to be_pre_op.

Fix Description:
1. pre_op: Adding missing dnatypes or replacing the magicregen
value with the current uid/gidNumber value is done at the
be_pre_op phase. Modify can use the entry set in pblock
with SLAPI_MODIFY_EXISTING_ENTRY (instead of getting the
entry by searching it internally). Also, if the operation
is a replication op, the pre_op is skipped.
2. The type of DNA plug-in is changed to bepreoperation.
3. post_op: Moving dna_config_check_post_op to BE_TXN_POST_OP.
If it is an internal operation, the dna post op is being
skipped to avoid self re-entrant deadlock.
4. Fixed memory leaks on DNA_NEEDS_UPDATE and an uninitialized
variable access.

I have one question. The plugin type in the DNA plugin entry has to be replaced:
-nsslapd-plugintype: preoperation
+nsslapd-plugintype: bepreoperation
The template-dnaplugin.ldif.in is being updated, but we need to support the upgrade case, as well. I was expecting dnaplugindepends.ldif (to be placed in /usr/share/dirsrv/updates) could do the job if we make a change like this:
changetype: modify
add: nsslapd-plugin-depends-on-type
nsslapd-plugin-depends-on-type: database
+-
+replace: nsslapd-plugin-depends-on-type
+nsslapd-pluginType: bepreoperation
But it does not update the already existing dse.ldif by "setup-ds.pl -u", which fails as follows:
[..] dna-plugin - dna_init: failed to register plugin
[..] - Init function "dna_init" for "Distributed Numeric Assignment Plugin" plugin in library "libdna-plugin" failed
[..] - Unable to load plugin "cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config"

Could there be an easy way to update a config param (without writing another perl script :)?

Replying to [comment:7 nhosoi]:

Thanks for your patience, Rich! I misunderstood your comments. I thought you were suggesting to move pre_op to betxn_pre_op, which I already tried and failed... :p Now, the pre_op_add/modify are moved to be_pre_op.

Sorry, I should have been more clear.

Fix Description:
1. pre_op: Adding missing dnatypes or replacing the magicregen
value with the current uid/gidNumber value is done at the
be_pre_op phase. Modify can use the entry set in pblock
with SLAPI_MODIFY_EXISTING_ENTRY (instead of getting the
entry by searching it internally). Also, if the operation
is a replication op, the pre_op is skipped.
2. The type of DNA plug-in is changed to bepreoperation.
3. post_op: Moving dna_config_check_post_op to BE_TXN_POST_OP.
If it is an internal operation, the dna post op is being
skipped to avoid self re-entrant deadlock.
4. Fixed memory leaks on DNA_NEEDS_UPDATE and an uninitialized
variable access.

I have one question. The plugin type in the DNA plugin entry has to be replaced:
-nsslapd-plugintype: preoperation
+nsslapd-plugintype: bepreoperation
The template-dnaplugin.ldif.in is being updated, but we need to support the upgrade case, as well. I was expecting dnaplugindepends.ldif (to be placed in /usr/share/dirsrv/updates) could do the job if we make a change like this:
changetype: modify
add: nsslapd-plugin-depends-on-type
nsslapd-plugin-depends-on-type: database
+-
+replace: nsslapd-plugin-depends-on-type
+nsslapd-pluginType: bepreoperation

did you mean

+replace: nsslapd-pluginType
+nsslapd-pluginType: bepreoperation

instead?

But it does not update the already existing dse.ldif by "setup-ds.pl -u", which fails as follows:
[..] dna-plugin - dna_init: failed to register plugin
[..] - Init function "dna_init" for "Distributed Numeric Assignment Plugin" plugin in library "libdna-plugin" failed
[..] - Unable to load plugin "cn=Distributed Numeric Assignment Plugin,cn=plugins,cn=config"

Could there be an easy way to update a config param (without writing another perl script :)?

It should work using the ldif - not sure what is going on.

Thank you so much, Rich!! That was it... ;_;) I tested the upgrade and verified nsslapd-pluginType was successfully replaced. The take 3 patch includes the change, too.

Pushed to master.

$ git merge work
Updating 836a31d..325abca
Fast-forward
ldap/admin/src/scripts/dnaplugindepends.ldif | 3 +
ldap/ldif/template-dnaplugin.ldif.in | 2 +-
ldap/servers/plugins/dna/dna.c | 1066 +++++++++++++++-----------
ldap/servers/slapd/operation.c | 9 +
ldap/servers/slapd/pagedresults.c | 3 +-
ldap/servers/slapd/slapi-plugin.h | 1 +
6 files changed, 614 insertions(+), 470 deletions(-)

$ git push
Counting objects: 33, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (16/16), done.
Writing objects: 100% (17/17), 5.01 KiB, done.
Total 17 (delta 13), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
836a31d..325abca master -> master

changeset:325abca/389-ds-base

Added initial screened field value.

Metadata Update from @rmeggins:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.2.11.a1

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/303

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