In the function urp_delete_operation, if to-be-deleted entry is found a tombstoned entry, the urp code is supposed to return -1 not to apply the delete in the backend code, but it should be treated as LDAP_SUCCESS not to return replay failure to the supplier. But the backend automatically sets -1 (Operation error) if the return value from the plugins.
I'm also concerned this error setting could cause the replication hang. Comment by the author.
/* * This operation won't be replayed. That is, this CSN won't update * the max csn in RUV. The CSN is left uncommitted in RUV unless an * error is set to op_result. Just to get rid of this CSN from RUV, * setting an error to op_result */ /* op_result = LDAP_SUCCESS; */
Bug description: When an urp resolution occurred as follows [] - urp_delete: Entry "nsuniqueid=<UNIQID>,uid=<UID>,o=<ORG>" is already a Tombstone. the operation should be skipped in the backend, but should return SUCCESS to the supplier. Otherwise, the supplier continues to send the relay and the replication stops there.
Fix description: This patch introduced SLAPI_PLUGIN_NOOP (-2) to the bepre and betxnpre plugin return value set (SLAPI_ PLUGIN_SUCCESS == 0; SLAPI_PLUGIN_FAILURE == -1). If SLAPI_ PLUGIN_NOOP is returned, the backend code skips the operation, but it returns SUCCESS. Note that urp is only executed on the replicated operation (not on the end user ones).
git patch file (master) 0002-Trac-Ticket-499-Handling-URP-results-is-not-corrrect.patch
I notice several places in the urp code where the op_result is set to a non-zero value and the rc plugin return code is set to SLAPI_PLUGIN_NOOP e.g. {{{ urp_modify_operation( Slapi_PBlock pb ) ... op_result= LDAP_NO_SUCH_OBJECT; 97 99 slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result); 98 rc= -1; / Must discard this Modification / 100 rc = SLAPI_PLUGIN_NOOP; / Must discard this Modification */ 101 slapi_log_error (slapi_log_urp, sessionid, 102 "urp_modify: No such entry\n"); }}}
and other places where SLAPI_PLUGIN_FAILURE is used e.g. {{{ urp_modrdn_operation( Slapi_PBlock pb ) 564 579 op_result= LDAP_OPERATIONS_ERROR; 565 580 slapi_pblock_set(pb, SLAPI_RESULT_CODE, &op_result); 566 rc = -1; 581 rc = SLAPI_PLUGIN_FAILURE; / Ignore this Operation */ }}}
Is this as expected?
Replying to [comment:6 rmeggins]:
I notice several places in the urp code where the op_result is set to a non-zero value and the rc plugin return code is set to SLAPI_PLUGIN_NOOP ...
Yes, I chose them depending upon how we want to continue/stop the replication. I thought in the modify case we could continue, but the modrdn case looks more serious and we should stop it there.
Reviewed by Rich (Thank you!!)
Pushed to master: commit d59f687
Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=918687
Metadata Update from @nhosoi: - Issue assigned to nhosoi - Issue set to the milestone: 1.3.0
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/499
If you want to receive further updates on the issue, please navigate to the github issue and click on subscribe button.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.