#49507 tombstone dn format is not technically correct
Opened 2 years ago by firstyear. Modified 4 months ago

Issue Description

Tombstone dn's are not technically correct:

nsuniqueid=c04c0107-e4a611e7-b5dae1e9-cbd6b25a,uid=testuser,ou=People,dc=example,dc=com

However, the parent object is ou=People,dc=example,dc=com

As a result this means the rdn is actually:

C3
      ID: 14; RDN: "nsuniqueid=c04c0107-e4a611e7-b5dae1e9-cbd6b25a,uid=testuser"; NRDN: "nsuniqueid=c04c0107-e4a611e7-b5dae1e9-cbd6b25a,uid=testuser"
14
      ID: 14; RDN: "nsuniqueid=c04c0107-e4a611e7-b5dae1e9-cbd6b25a,uid=testuser"; NRDN: "nsuniqueid=c04c0107-e4a611e7-b5dae1e9-cbd6b25a,uid=testuser"
P14
      ID: 3; RDN: "ou=People"; NRDN: "ou=people"

This is not really compliant to ldap standards. We should investigate if it's possible to make this compliant as a multivalued rdn IE:

nsuniqueid=c04c0107-e4a611e7-b5dae1e9-cbd6b25a+uid=testuser,ou=People,dc=example,dc=com

This would required a detailed investigation to ensure that replication conflict management, and tombstone management are able to work with the existing format and corrected format.


Metadata Update from @firstyear:
- Custom field component adjusted to None
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None
- Custom field type adjusted to None
- Custom field version adjusted to None
- Issue set to the milestone: FUTURE

2 years ago

I implemented a solution for this, triggered by the work on conflict resolution. A design doc is here:

http://www.port389.org/docs/389ds/design/use-correct-tombstone-rdns.html

A patch will follow

Design doc looks great, nice job!

Patch review, only one very minor issues

entryrdn_rename_subtree() - indentation off around if/else
entryrdn_rename_subtombstones() to TRACE logging calls, one has the old function name.

The rest looks good!

I'll let @tbordaz have a review, and he can ack if he agrees

updated the doc with a missing operation and a discussion of mixed version topologies

the patch needs to be extended

@lkrispen, the design doc and patch are looking good to me.
Very minor comments:
in moddn_remove_children_from_cache
we remove child DNs from dncache. Why is it conditional to entryrdn_get_switch ?

in ldbm_delete
Slapi_RDN *sdn is not used

in ldbm_entryrdn
log msg with 'entryrdn_rename_subtree' in 'entryrdn_rename_subtombstones'

A question regarding DEL entry with only tombstone children.
It moves the children tombstones under the deleted parent with the update of entryrdn.
How was it done before ?

@lkrispen, the design doc and patch are looking good to me.
Very minor comments:
in moddn_remove_children_from_cache
we remove child DNs from dncache. Why is it conditional to entryrdn_get_switch ?

the code is taken from the code exising in ldbm_modrdn and put into a function, so it is a s before. I think it is to be seen with moddn_get_children where also the entryrdn switch triggers execution.
In fact the tombstone children are only properly renamed if the entryrdn switch is on. And I think it is now always oo, but if you want to work tombstone children rename properly with teh switch to off I will have to invest time

in ldbm_delete
Slapi_RDN *sdn is not used

yep

in ldbm_entryrdn
log msg with 'entryrdn_rename_subtree' in 'entryrdn_rename_subtombstones'

good catch, will fix

A question regarding DEL entry with only tombstone children.
It moves the children tombstones under the deleted parent with the update of entryrdn.
How was it done before ?

It does not really move them, it updates the parent records with the proper new parent rdn. It was not done at all before and that is why we had the problems reported in ticket 49615

@lkrispen, please do not make it work with 'entryrdn switch: off', it is almost a deprecated mode.
My remark was more about the DN cache, AFAIK DN cache should not be impacted with that switch on/off.

I missed that the culprit of the bug was missing update of parent records. So it explains why this code is new :)

So regarding that patch you have my ACK

The tickets #49615, #49616 and #49507 should be handled together, but they didn't get attention for quite some time.
A problem might be that the format changes and we need to be carefule when to introduce it.

Please keep them open and I will look into them again.

Login to comment on this ticket.

Metadata