if 0

This dead code is also the only user of ldbm_back_next_search_entry_ext with useextension set to 1.
otherwise:

 ldbm_back_next_search_entry()
     ldbm_back_next_search_entry_ext(pb,0)

so we could merge these two and simplify by eliminating use_extension

Issue linked to Bugzilla: Bug 1859282

https://pagure.io/389-ds-base/pull-request/51218

Commit c6aae1e5 relates to this ticket

This broke automember tests and some basic tests: https://fedorapeople.org/groups/389ds/ci/nightly/2020/07/28/report-389-ds-base-1.4.4.4-20200727gitc6aae1e.fc32.x86_64.html

Turns out we do use ldbm_back_entry_release() afterall in opshared.c:

static void
cache_return_target_entry(Slapi_PBlock *pb, Slapi_Backend *be, Slapi_Operation *operation)
{
    if (operation_get_target_entry(operation) && be->be_entry_release) {
        (*be->be_entry_release)(pb, operation_get_target_entry(operation));
        operation_set_target_entry(operation, NULL);
        operation_set_target_entry_id(operation, 0);
    }
}

Adding that code back...

Ok, I missed this usage when creating the ticket, but I think it is still not clean. The call is in opshared.c when handling the return code of be_search - and it does operate on the entry cache, which it should not know of.
Instead of dealing with the ec when handling rc of be_search I think it should be done inside be_search before returning, and could use functions directly instead of a pb function

Sorry, yeah I'm trying to clean it all up - I am not adding it back as it was...

I found some other pblock params that are not used (or not used as intended):

• SLAPI_PLUGIN_PRECEDENCE
• SLAPI_PLUGIN_POSTSTART_FN
• SLAPI_PLUGIN_EXT_OP_BACKEND_FN
• SLAPI_PLUGIN_MR_FLAGS
• SLAPI_PLUGIN_MR_NORMALIZE

But I will open another ticket to look at these...

Ok, I missed this usage when creating the ticket, but I think it is still not clean. The call is in opshared.c when handling the return code of be_search - and it does operate on the entry cache, which it should not know of.
Instead of dealing with the ec when handling rc of be_search I think it should be done inside be_search before returning, and could use functions directly instead of a pb function

I will look into this, thanks

So we call cache_return_target_entry() in opshared.c when be_search fails, but also when it succeeds. When it succeeds we do checks for psearches, and how results are sent back, and depending on the error and how things happen we "release" the target or we don't. It's not straightforward.

To try and move this "cache returning" would require changes to the search function and to the result handling. I feel, in this ticket, we would be trying to fix something that is not broken, and the entry cache is fragile enough. Maybe if we rework the entry cache at a later date we can look into this when we go to make bigger changes to the design?

Anyway I was able to do more cleanup, and I'm going to update the PR...

https://pagure.io/389-ds-base/pull-request/51221

To try and move this "cache returning" would require changes to the search function and to the result handling. I feel, in this ticket, we would be trying to fix something that is not broken, and the entry cache is fragile enough.

yes, there is no real need to do it, can be part of another cleanup

Commit b7865bf1 relates to this ticket

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

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.