[
https://issues.apache.org/jira/browse/NIFI-16359?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jannik Rebmann updated NIFI-16359:
----------------------------------
Summary: Git Flow Registry Clients: Cache commit listing per (path, branch)
to remove per-process-group API load (was: Port GitHubFlowRegistryClient
API-call optimizations (NIFI-14837) to GitLabFlowRegistryClient)
> Git Flow Registry Clients: Cache commit listing per (path, branch) to remove
> per-process-group API load
> -------------------------------------------------------------------------------------------------------
>
> Key: NIFI-16359
> URL: https://issues.apache.org/jira/browse/NIFI-16359
> Project: Apache NiFi
> Issue Type: Improvement
> Components: Flow Versioning
> Affects Versions: 2.12.0
> Reporter: Jannik Rebmann
> Priority: Major
>
> h2. Summary
> GitLabFlowRegistryClient against a self-hosted GitLab instance generates high
> load on the
> GitLab server as the number of versioned process groups grows. The load does
> not come from
> one expensive operation but from a per-process-group multiplier on the
> version-check path,
> compounded by a GitLab-specific full-history listing. This ticket proposes
> removing that
> multiplier at the shared base-class level so all Git-based registry clients
> benefit.
> h2. Environment / Observation
> Self-hosted GitLab, many versioned process groups. Our infrastructure team
> flagged a high
> volume of GitLab REST API calls (repository/commits, repository/tree,
> repository/files)
> originating from NiFi. The volume scales with the number of versioned process
> groups, even
> when most of them are bound to the *same* flow at the *same* version.
> h2. Root cause
> The dominant driver is a per-process-group multiplier, not the cost of a
> single listing:
> 1. The periodic "up to date" check and version listing run {*}per process
> group{*}. In
> AbstractGitFlowRegistryClient, getLatestVersion(...), getFlowVersions(...)
> and the
> latest-version comparison in getFlowContents(...) all call
> repositoryClient.getCommits(filePath, branch). With N process groups bound to
> the same
> flow (identical filePath+branch), each cycle issues N identical
> repository/commits
> requests. There is no deduplication across process groups.
> 2. repositoryClient is a singleton: getRepositoryClient(context) builds the
> client once
> (guarded by clientInitialized) and reuses the volatile instance. A cache
> placed on the
> base class is therefore shared across every process group using the component
> - i.e. the
> multiplier can be removed with a shared, in-memory cache.
> 3. GitLab-specific amplification: GitLabRepositoryClient.getCommits(path,
> branch) uses the
> gitlab4j List overload that delegates to Pager.all(), which pages the
> *entire* commit
> history for the path (the client sets per_page=100). Each of the N calls
> above is thus
> itself several page requests.
> 4. Content resolution (repository/tree, repository/files) is likewise
> performed per process
> group when materializing a version.
> h2. Relationship to NIFI-14837 / PR #10186
> NIFI-14837 (fixed in 2.6.0, PR #10186) improved the GitHub client by (a)
> limiting the commit
> listing to the first page and (b) adding an in-memory SHA->commit-detail
> cache. Two points:
> * Those changes were applied only to GitHubRepositoryClient. GitLab has
> neither, and
> additionally pages the full history (point 3 above).
> * PR #10186 reduced GitHub load from O(processGroups x commits) to
> O(processGroups): it
> removed the full-history paging and the per-commit N+1 detail fetch. It did
> NOT remove the
> per-process-group multiplier - GitHubRepositoryClient.getCommits(path,
> branch) still issues
> a live getRef + first-page listing on every call, with no (path, branch)
> cache and no TTL.
> This simply was not visible at a small number of process groups. So GitHub
> scales linearly
> with process-group count as well.
> * The SHA->commit-detail cache is not the load-bearing part for GitLab.
> gitlab4j's commit
> listing returns fully-populated Commit objects (author, message,
> committed_date inline),
> so there is no per-commit detail call to eliminate. Porting that cache 1:1 to
> GitLab would
> add a dependency and complexity without reducing API calls.
> h2. Proposed solution
> 1. Primary, general fix - cache the commit listing at the base class. Add a
> short-TTL,
> size-bounded in-memory cache keyed by (filePath, branch) in
> AbstractGitFlowRegistryClient,
> wrapping the getCommits calls in getLatestVersion / getFlowVersions /
> getFlowContents.
> This collapses the per-process-group calls to one remote call per (flow,
> branch) per TTL
> window and benefits all Git clients (GitHub, GitLab, Bitbucket, Azure
> DevOps). TTL bounds
> the staleness for detecting a *remote* new version; invalidate eagerly on
> local writes
> (createContent, deleteContent) and on client (re)initialization so locally
> committed
> versions are visible immediately. TTL and size hard-coded initially; can be
> promoted to
> properties in a follow-up.
> 2. GitLab-specific - bound the commit listing to the first page (a small
> COMMIT_PAGE_SIZE)
> instead of Pager.all(), to stop full-history paging and cap payload size,
> mirroring the
> listing limit from PR #10186.
> 3. Optional follow-up - immutable content cache. For the repository/tree and
> repository/files load, add a cache keyed by (commitSha, path). Content at a
> fixed SHA is
> immutable, so this can be cached without a TTL.
> h2. Scope question for reviewers
> The primary fix lives in the shared base class and benefits all four Git
> clients, so this
> ticket's scope effectively broadens beyond GitLab. Should NIFI-16359 own the
> base-class
> caching (with GitLab as the motivating case), or should the base-class change
> be split into a
> dedicated framework ticket that per-client tickets reference? Input welcome
> before
> implementation.
> h2. References
> NIFI-14837, PR [https://github.com/apache/nifi/pull/10186]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
