Skip to content

fix(perf,tenancy): fix /environments/enabled CPU storm + restore per-repo scoping on web reads#1179

Merged
krusche merged 1 commit into
stagingfrom
fix/env-enabled-perf-and-repo-filter
Jul 3, 2026
Merged

fix(perf,tenancy): fix /environments/enabled CPU storm + restore per-repo scoping on web reads#1179
krusche merged 1 commit into
stagingfrom
fix/env-enabled-perf-and-repo-filter

Conversation

@krusche

@krusche krusche commented Jul 3, 2026

Copy link
Copy Markdown
Member

Prod incident 2026-07-03

App-server pinned ~140% CPU, /api/environments/enabled ~6s TTFB, and pipeline/PR/environment lists showed all repositories' data.

Root causes

  1. Write-in-read: getAllEnvironments/getAllEnabledEnvironments called unlockExpiredEnvironments() (a @Scheduled @Transactional write) inside the per-row .map() → N write txns + auto-flush/dirty-check churn per request (confirmed via prod thread dump).
  2. Missing indexes: per-environment latest-deployment + release-candidate lookups were sequential scans.
  3. Tenant filter off on web reads: the v1.7.0 RepositoryFilterAspect → RepositoryFilterTransactionConfig refactor only enables gitRepositoryFilter on tx-manager-created EntityManagers. Under OSIV (default on) the request tx reuses the OSIV-opened EM, so the filter never enabled for web reads → cross-repo leak (invisible in single-repo staging).

Fixes (quickfix — option A)

  • Remove unlockExpiredEnvironments() from the read maps (scheduler still runs every 60s) + drop the unused field.
  • V56 indexes (already created live on prod; IF NOT EXISTS).
  • Re-enable the filter for web requests: RepositoryInterceptor enables gitRepositoryFilter on the request's OSIV EntityManager, ordered after OSIV via order(Integer.MAX_VALUE). Logs a warning if the EM isn't bound (staging observability).
  • Persist the NATS throttle (MAX_ACK_PENDING written from repo vars = 20; ACK_WAIT=600) so a deploy won't revert to 500 and re-trigger the redelivery storm.

Follow-up (no time pressure)

Migrate to explicit per-query repository_id filtering and remove the Hibernate-filter magic (option B).

Verification plan

Merge → staging deploy → confirm CPU normal, /enabled fast, no gitRepositoryFilter not enabled warnings, endpoints scoped correctly → cut 1.7.2 → prod → verify.

…-repo scoping on web reads

Prod incident 2026-07-03: app-server pinned ~140% CPU, /api/environments/enabled ~6s TTFB,
and the pipeline/PR/environment lists showed data from ALL repositories.

Root causes:
- getAllEnvironments/getAllEnabledEnvironments called EnvironmentScheduler.unlockExpiredEnvironments()
  (a @scheduled @transactional WRITE) inside the per-row .map(), turning each read into N write
  transactions + auto-flush/dirty-check churn.
- Missing indexes made the per-environment latest-deployment and release-candidate lookups
  sequential scans.
- The v1.7.0 refactor (RepositoryFilterAspect -> RepositoryFilterTransactionConfig) only enables
  Hibernate's gitRepositoryFilter on transaction-manager-created EntityManagers. Under
  Open-Session-In-View the request transaction reuses the OSIV-opened EM, so the filter never
  enabled for web reads -> every tenant query leaked across repositories (invisible in
  single-repo staging).

Fixes:
- Remove the unlockExpiredEnvironments() calls from the read maps (the scheduler still runs every
  60s on its own) and drop the now-unused field.
- V56: idx_deployment_env_created, idx_helios_deployment_env_created, idx_release_candidate_repo_sha
  (already created live on prod during the incident; IF NOT EXISTS).
- Re-enable gitRepositoryFilter for web requests: RepositoryInterceptor enables it on the request's
  (OSIV) EntityManager, registered after OSIV via order=Integer.MAX_VALUE.
- Persist the NATS throttle: write NATS_CONSUMER_MAX_ACK_PENDING from repo vars so a deploy no
  longer reverts it to 500 (with ACK_WAIT=600) and re-triggers the redelivery storm.

Follow-up (no time pressure): migrate to explicit per-query repository filtering and remove the
Hibernate-filter magic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@krusche krusche requested a review from a team as a code owner July 3, 2026 15:27
Copilot AI review requested due to automatic review settings July 3, 2026 15:27
@codacy-production

codacy-production Bot commented Jul 3, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Coverage 11.11% diff coverage · -0.04% coverage variation

Metric Results
Coverage variation -0.04% coverage variation (-1.00%)
Diff coverage 11.11% diff coverage

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (9fada37) 15430 7601 49.26%
Head commit (8827512) 15441 (+11) 7600 (-1) 49.22% (-0.04%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#1179) 18 2 11.11%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Mitigates a production performance/tenancy incident by removing a write-path from /api/environments/enabled, adding DB indexes for the hot queries, and restoring per-repository scoping for web reads under OSIV by explicitly enabling the Hibernate gitRepositoryFilter on the request-bound EntityManager.

Changes:

  • Adds V56 DB indexes to avoid sequential scans for latest-deployment and release-candidate lookups.
  • Removes unlockExpiredEnvironments() calls from environment list read-mappers to prevent per-row write transactions.
  • Ensures web requests enable gitRepositoryFilter under OSIV by ordering and enhancing RepositoryInterceptor; persists NATS MAX_ACK_PENDING into the deploy workflow env.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server/application-server/src/main/resources/db/migration/V56__add_environment_deployment_indexes.sql Adds indexes for the environments “enabled” hot path queries.
server/application-server/src/main/java/de/tum/cit/aet/helios/filters/RepositoryInterceptor.java Enables gitRepositoryFilter on the OSIV-bound EntityManager during web requests.
server/application-server/src/main/java/de/tum/cit/aet/helios/environment/EnvironmentService.java Removes scheduler-triggered writes from read endpoints to prevent CPU storms.
server/application-server/src/main/java/de/tum/cit/aet/helios/config/SecurityConfig.java Orders the repository interceptor after OSIV so the request EntityManager is bound before enabling the filter.
.github/workflows/deploy_docker.yml Persists NATS MAX_ACK_PENDING into generated .env during deploy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +43 to 55
final EntityManagerHolder holder =
(EntityManagerHolder) TransactionSynchronizationManager.getResource(entityManagerFactory);
if (holder != null) {
holder
.getEntityManager()
.unwrap(Session.class)
.enableFilter("gitRepositoryFilter")
.setParameter("repository_id", repositoryId);
} else {
log.warn(
"gitRepositoryFilter not enabled for repository {}: no EntityManager bound at preHandle",
repositoryId);
}
@krusche krusche merged commit 911e04f into staging Jul 3, 2026
24 checks passed
@krusche krusche deleted the fix/env-enabled-perf-and-repo-filter branch July 3, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants