#1126 Reuse cache_req() in responder code

Created 5 years ago by jzeleny
Modified 7 months ago

The function nss_cmd_getpwnam_search() has several slightly modified copies through the code in responders. It would be nice to create one function (or macro) which would be generic and used in all parts of the code instead of those copies.

There are a number of improvements we should work on while fixing this. In particular, we should seek to resolve the performance issue identified in bug #843.

The current approach behaves as follows (I will use {{{getpwnam()}}} as the representative function, though all are equivalent).

The NSS responder will check the cache for the first domain in the list. If it is empty or expired, the Responder will query the Data Provider. If the Data Provider returns DP_ERR_OK, the cache is checked again. If the entry is still not present, it advances to the next domain. This pattern repeats.

The problem with this approach is that it means that for a configuration of N domains, if a user is found in the last domain in the list, SSSD will go to the server N-1 times before ever checking the cache of the correct domain.

The proposed new approach is for us to search each domain's cache first. If any domain has the requested, non-expired entry, it should be returned immediately. If all domains lack the entry, only then should we begin to iterate through them. For NSS lookups, we should NOT limit ourselves only to the domain with an expired entry for the user, because we need to be able to catch new users being added to earlier domains. So in the complete cache-miss case, we will still iterate through all domains, but a cache-hit will not result in network activity.

For PAM, we will take an additional shortcut. Since it is reasonable to assume that the login program will have made a {{{getpwnam()}}} call prior to starting the PAM conversation, we will limit our mandatory {{{initgroups()}}} lookup to only domains with an expired entry (if one exists, otherwise we will try them in series).

blocking: => 843

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.9.0 NEEDS_TRIAGE

Fields changed

blocking: 843 =>

"Nice to have" for 1.9.

milestone: SSSD 1.9.0 NEEDS_TRIAGE => SSSD 1.9.0

Fields changed

feature_milestone: =>
priority: major => minor

Fields changed

rhbz: => 0

Implementing this sooner will ease work for Pavel. I'm going to squash this one out while he's doing prep work to help him.

owner: somebody => jhrozek

When implementing this, use the code written for getserv_send(), lookup_service_step() and lookup_service_done() from nssrv_services.c as the guideline. That approach will address Bug #843 as well.

Fields changed

proposed_priority: => Nice to have

Fields changed

milestone: SSSD 1.9.0 => Temp milestone

Fields changed

blocking: => 843

Fields changed

blocking: 843 => #843

Moving all the features planned for 1.10 release into 1.10 beta.

milestone: Temp milestone => SSSD 1.10 beta

Fields changed

selected: => Not need

Fields changed

design: =>
design_review: => 0
fedora_test_page: =>
selected: Not need => Want

Fields changed

selected: Want => Not need

Fields changed

review: => 1

Fields changed

milestone: SSSD 1.10 beta => SSSD 1.11 beta

Putting up to NEEDS_TRIAGE so we can discuss the priority on the next meeting.

changelog: =>
keywords: => localaccounts
milestone: SSSD 1.13 beta => NEEDS_TRIAGE

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.12 beta

Would be nice to have for local users support.

milestone: SSSD 1.12 beta => SSSD 1.13 beta

Fields changed

mark: => 0

We should use Pavel's cache request in the NSS responder instead.

keywords: localaccounts =>
priority: minor => critical

Migrating to Pavel's cache_req should be done in 1.14, it's too late for 1.13 now

milestone: SSSD 1.13 beta => SSSD 1.14 beta
sensitive: => 0

Fields changed

owner: jhrozek => pbrezina

Fields changed

blockedby: => #2896
summary: Reuse nss_cmd_getpwnam_search() in responder code => Reuse cache_req() in responder code

Fields changed

blockedby: #2896 => #2869

Some patches are already landing on the list, so I'm moving this ticket to 1.14 alpha.

milestone: SSSD 1.14 beta => SSSD 1.14 alpha

sudo is done, but more work in the other responders is needed.

milestone: SSSD 1.14 alpha => SSSD 1.15 beta

Fields changed

blockedby: #2869 => #2869, #3151

Fields changed

milestone: SSSD 1.16 beta => SSSD 1.15 Alpha

Fields changed

patch: 0 => 1

Fields changed

milestone: SSSD 1.15.0 => SSSD 1.15 Beta

Fields changed

milestone: SSSD 1.16 Beta => SSSD 1.15.1

8 months ago

Metadata Update from @jzeleny:
- Issue assigned to pbrezina
- Issue marked as blocked by: #2228
- Issue marked as blocked by: #843
- Issue marked as depending on: #2869
- Issue marked as depending on: #3151
- Issue set to the milestone: SSSD 1.15.1

pam fixes:

master:
c99bcc9
5aaaf08
ed891c0
189db53

Edited 7 months ago by jhrozek
7 months ago

Metadata Update from @jhrozek:
- Custom field blocking reset (from #843)
- Custom field design_review reset (from 0)
- Custom field mark reset (from 0)
- Custom field patch adjusted to on (was: 1)
- Custom field review adjusted to on (was: 1)
- Custom field sensitive reset (from 0)
- Custom field testsupdated reset (from 0)
- Issue unmarked as blocking: #2869
- Issue unmarked as blocking: #3151
- Issue unmarked as depending on: #2228
- Issue unmarked as depending on: #843

7 months ago

Metadata Update from @jhrozek:
- Custom field design_review reset (from false)
- Custom field mark reset (from false)
- Custom field sensitive reset (from false)
- Custom field testsupdated reset (from false)

7 months ago

Metadata Update from @jhrozek:
- Custom field design_review reset (from false)
- Custom field mark reset (from false)
- Custom field sensitive reset (from false)
- Custom field testsupdated reset (from false)
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

Login to comment on this ticket.

task

SSSD

1.6.3

Not need

false

on

0

false

on

false

false

#2869, #3151

cancel