#51069 Ticket 51068 - deadlock when updating the schema
Closed by spichugi. Opened by tbordaz.
tbordaz/389-ds-base ticket_51068  into  master

Download 51069.patch

Bug Description:
It exists a 3 threads deadlock scenario. It involves state change plugins when it
calls schema_changed_callback. So the trigger is a change of schema (direct or via
replication). The scenario is
MOD(cn=schema) hold StateChange lock wait for vattr lock
SRCH hold vattr lock wait for DB page
MOD hold DB page wait for StateChange lock

Fix Description:
Statechange lock protects the list of registered callbacks.
lock is a mutex where actually registration of callback is only done
at startup. Later the list is only lookup.
Making statechange lock a rwlock suppresses the deadlock scenario
as MODs will only acquire in read StateChange lock.
It should also improve performance as at the moment all MODs are serialized
on that lock

https://pagure.io/389-ds-base/issue/51068

Reviewed by: ?

Platforms tested: 30

Flag Day: no

Doc impact: no

Seems quite straight forward to me, ack.

@firstyear, thanks for your review. The fix fixes the deadlock but @vashirov noticed that the MOD schema was waiting very long to acquire the RW vattr lock (hold by the searches). @mreynolds improved the patch and a new PR is likely coming.. :)

Is that related to linux's write/reader policies on pthread locks?

Which policy are you referring about ? My understanding is that writer (schema update) prevents new readers (search threads) but current readers have to complete before the writer can proceed. So a writer has (should) the priority over the readers but still needs to wait.

@tbordaz have a read of this https://fy.blackhats.net.au/blog/html/2018/10/19/rust_rwlock_and_mutex_performance_oddities.html?highlight=rwlock it may help to explain what I mean.

rebased onto 9e6cccd5d5ff530835aff136cf4abf2ca55ef326

rebased onto d593eea8cfc0a5ecf602248511e3ec33281f0425

@mreynolds, @firstyear. I updated the original fix setting the vattr rwlock priority to writers.
It works to both prevent the deadlock and allow immediate update of schema.
Would you please review the new patch ?

rebased onto 1b7b12e2f7a8f3004348dd1a51788e81d02dfeed

Pull-Request has been merged by tbordaz

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 pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4122

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

Metadata