memberof: derive membership graphs once per transaction - #9051
Draft
karlg100 wants to merge 10 commits into
Draft
Conversation
A raw LDB rename changes the entry DN but leaves member and memberOf values that refer to the old DN. That disconnects otherwise valid direct and transitive membership edges. Add a rename handler that finds references to the old DN, forwards the rename, then replaces only the affected member and memberOf values while preserving controls and response data. Dependency: none. This is independently useful and is required before the same-GID sysdb rename change. Related: SSSD#9049
The rename reference search accumulates an unbounded number of matching entries. Keep its counters in size_t and reject growth at SIZE_MAX before computing num_entries + 1 for talloc_realloc(). This prevents integer wrap from turning a large result count into an undersized allocation followed by an out-of-bounds write. Dependency: memberof rename support from the preceding patch.
When a group arrives under a new name with a GID already present in the cache, deleting and recreating the old entry discards membership state and forces expensive reconstruction. Treat the entry as a rename only when stable identity attributes agree. Prefer SID or UUID, fall back to originalDN, rename the cached DN, remove the stale timestamp-cache key, and then store the incoming attributes. After ldb_rename(), explicitly replace the stored name. Searches in the open transaction can already expose the new RDN and otherwise make the generic no-op comparison suppress this required write. Dependency: memberof rename support, because all member and memberOf references must follow the renamed DN. Related: SSSD#9049
Exercise a same-identity, same-GID group rename while the group is both a child and a parent in a nested graph. Query member and memberOf explicitly, then verify the old name disappears, direct and inherited references use the new DN, and initgroups returns the renamed group without stale membership. Dependencies: memberof rename support and the verified same-GID sysdb rename patches.
The legacy rebuild path rewrites memberOf and memberUid on every user and group even when the derived value sets are identical. In large caches those no-op LDB writes dominate transaction time and can push the backend into the watchdog window. Hash the original and rebuilt value sets, suppress duplicate memberUid values, and skip the modify request when neither derived attribute changed. Dependency: none. This optimization is independent of the transactional graph engine and can be cherry-picked on its own. Related: SSSD#9049
The legacy module performs recursive reverse-membership searches and derived-attribute writes while processing each changed edge. Dense caches turn large replacements or same-GID refreshes into repeated graph walks that can occupy the backend past its watchdog interval. Introduce a transaction-scoped graph engine. It snapshots graph-relevant cache data on the first relevant write, journals direct ghost and rename state, condenses cycles with strongly connected components, derives ancestor/memberUid/ghost sets once at prepare_commit, and writes only changed derived attributes. Public ghost writes remain on forwarded requests, preserving lower-module schema and modify validation. The transaction snapshot also tracks the public ghost attribute so successful write sequences compute derived deltas from the same transient state stored below memberof. A later patch adds the separate failure latch for rejected forwarded writes. Metadata-only cache refreshes bypass graph setup. A persistent migration marker distinguishes direct ghost values from inherited legacy values, and SSSD_MEMBEROF_LEGACY retains an operational fallback during rollout. LDB remains the storage engine. This implementation stays in one compilation unit because its snapshot, journal, cycle handling, and flush stages share private invariants; splitting them would create unbuilt or partially functional intermediate commits. Dependency: the memberof rename handler patch, used as the legacy fallback when the transactional engine is disabled. Fixes the repeated graph-work mechanism described in SSSD#9049
Graph snapshots and work queues derive allocation sizes from directory-controlled value counts. Check addition, capacity doubling, element-size multiplication, LDB unsigned cardinality limits, and value_length + 1 before allocating or copying. Also reject non-empty values with a NULL source pointer and make the final size_t-to-unsigned assignment explicit after validation. Dependency: the transactional memberof graph engine. This patch is intentionally separate so the allocation and cardinality protections can receive focused review and fast-track treatment.
The transaction graph must describe the state accepted by lower LDB modules. Forward graph-relevant writes through completion callbacks and apply add, modify, delete, and rename journal changes only after the lower request succeeds. Expected duplicate additions and absent deletions now leave the journal unchanged and do not poison the transaction. If an internal graph update fails after a successful database write, latch the failure and reject prepare_commit because the database and journal can no longer be reconciled safely. Dependencies: the transactional memberof graph engine and its lower-request callback infrastructure.
Add focused coverage for repeated empty transaction lifecycles, migration-marker rollback, add/modify/delete/rename replay within one transaction, direct and inherited ghost provenance, and a 3,500-member replacement mixed with metadata-only updates. The scale case verifies direct member values, derived memberUid values, and retained/removed memberOf edges after reducing the group to four members. The ghost case replaces direct ghost values, verifies that inherited values stay out of ghostDirect, and confirms that deleting an absent ghost is rejected by lower LDB while transaction cancellation preserves existing values. Dependencies: the transactional memberof graph engine and the rename test patch, which provides the shared test_message_has_value() helper. These tests do not require the failed-write latch patch.
Exercise an absent member deletion and a duplicate member addition inside one transaction, then perform a valid addition and commit. The failed no-op writes must be rejected by the lower LDB layer without changing or poisoning the transaction journal. The later successful write must still update direct membership, reverse memberOf, and derived memberUid state. This covers the asynchronous lower-request callback while preserving the expected sysdb transaction semantics.
Author
|
waiting to submit this from draft until #9050 has been reviewed and accepted. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The memberOf plugin currently performs recursive reverse-membership searches
and derived-attribute writes while processing individual graph changes.
Dense caches and large group replacements can therefore trigger repeated
graph walks and large numbers of redundant writes within one transaction.
This can keep the backend occupied beyond its watchdog deadline and block
identity and login requests.
Solution
Introduce a transaction-scoped membership graph engine that:
memberUid, and ghost sets once at prepare-commit;Expected lower-layer errors, such as duplicate member additions and removal of
absent members, leave the graph journal unchanged and do not poison the
transaction. An internal graph failure after a successful database write
still prevents commit because the database and journal can no longer be
safely reconciled.
Allocation growth, value cardinality, and integer conversions are explicitly
bounded before allocating or copying directory-controlled values.
Tests
Add focused coverage for:
memberOf, and derivedmemberUidresults.Scope
This addresses the repeated membership-graph work and large-group replacement
path described in #9049.
It depends on the rename support proposed in #9050. It does not include the
separate NSS mmap cache-warming optimization.
Validation
on the RHEL 9 and RHEL 10 investigation branches.
against the exact master-rebased series before marking this PR ready.
Refs #9049
Depends on #9050