With the introduction of backend transaction plugins the MemberOf Plugin global lock (memberof_lock()/memberof_unlock()) might no longer be needed.
The lock was introduced to keep the member updates synchronized, as under concurrent load it was possible for members to not have the correct memberOf value(either missing or incorrect, etc). This should no longer be an issue with the plugin now being a backend transaction plugin, but it still needs to be investigated.
Per triage, push the target milestone to 1.3.6.
Metadata Update from @nhosoi: - Issue set to the milestone: 1.3.6.0
Metadata Update from @mreynolds: - Issue close_status updated to: None - Issue set to the milestone: 1.3.7 backlog (was: 1.3.6.0)
Metadata Update from @mreynolds: - Issue set to the milestone: 1.4 backlog (was: 1.3.7 backlog)
Customer hitting a dead lock that would be fixed if this ticket is applied. Starting investigation...
Metadata Update from @mreynolds: - Custom field reviewstatus adjusted to None - Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1501058
Metadata Update from @mreynolds: - Issue assigned to mreynolds
I agree that if this lock to prevent concurrent updates, it is no required today as memberof is a BETXN plugins. Need to confirm the purpose of that lock.
This patch is based off of 1.3.6 code -so indentation might be off, but it will be fixed when frontporting to 1.3.7 and up
<img alt="0001-Ticket-48235-Remove-memberOf-global-lock.patch" src="/389-ds-base/issue/raw/files/0ee8df69200a74c3b3bfa84ceb7ab20c1f4ada1b8ec4da87496e3cae13141409-0001-Ticket-48235-Remove-memberOf-global-lock.patch" />
Metadata Update from @mreynolds: - Custom field reviewstatus adjusted to review (was: None) - Issue set to the milestone: 1.3.6.0 (was: 1.4 backlog)
The fix looks very good to me. ACK. I think you can get rid of 'plhash.h' in memberof.c now.
Metadata Update from @tbordaz: - Custom field reviewstatus adjusted to ack (was: review)
I agree, this looks really good. Thanks Mark!
@tbordaz - we still need that header in memberof.c
FYI, I also ran all the CI tests that use memberof and they all passed.
b761460..184b8a1 389-ds-base-1.3.6 -> 389-ds-base-1.3.6
0658297..8915d8d 389-ds-base-1.3.7 -> 389-ds-base-1.3.7
2b8b6db..df4492b master -> master
Metadata Update from @mreynolds: - Issue close_status updated to: fixed - Issue status updated to: Closed (was: Open)
Fix cherry-pick errors
34a184a..3eb443b master -> master
8915d8d..4471b73 389-ds-base-1.3.7 -> 389-ds-base-1.3.7
e6ac5ec..80c8795 389-ds-base-1.3.6 -> 389-ds-base-1.3.6
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/1566
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.